Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and improvement in the RouteNode tree router implementationFix/match route #54

Merged
merged 10 commits into from
Sep 29, 2024

Conversation

aristotelesbr
Copy link
Owner

Description

This PR resolves conflicts between dynamic parameters and static routes.

Key changes:

  • Refactor of params: The argument params = {} was changed to params: {}, using keyword arguments.
  • Handling of multiple dynamic routes: The dynamic_children is now a list, allowing multiple dynamic routes to coexist at the same level without overwriting each other.
  • Clear separation between static and dynamic routes: Static routes are stored in static_children, while dynamic ones are handled separately in dynamic_children.
  • Recursive matching: The match_route method was adjusted to perform recursive route matching, ensuring both static and dynamic routes are checked efficiently.

Motivation:

  • Resolution of conflicts between dynamic parameters and static routes: The separation of static and dynamic routes solves parameter overwriting issues, ensuring each route is handled correctly.

  • Performance and scalability: The new implementation optimizes how routes are stored and matched, allowing greater flexibility and improved performance in scenarios with a large number of routes.

Next steps:

Benchmark tests: Benchmark tests are still pending to assess the impact of the refactor on performance. I believe the results may not be as encouraging as the initial ones, but it’s important to run them to ensure the refactor does not negatively affect performance. 🙃

@aristotelesbr
Copy link
Owner Author

I ran the initial tests and noticed that the use of find slightly impacted the RPS performance.

dynamic_node = current_node.dynamic_children.find { |node| node.param_key == param_sym }

This was actually expected. To address this, I changed the structure of dynamic_children to use a hash instead of an array and replaced the linear search with find by directly accessing dynamic_children. Now that it’s a hash, this significantly improved the route matching performance

current_node.dynamic_children[param_sym] ||= RouteNode.new
dynamic_node = current_node.dynamic_children[param_sym]

rps

The implementation still proves to be efficient and reliable for handling a high volume of requests. 🙂

@aristotelesbr aristotelesbr merged commit ce3d903 into development Sep 29, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant