-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Improve router performance with dedicated child types #1689
Improve router performance with dedicated child types #1689
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1689 +/- ##
==========================================
+ Coverage 85.19% 85.30% +0.11%
==========================================
Files 29 29
Lines 1986 2001 +15
==========================================
+ Hits 1692 1707 +15
Misses 186 186
Partials 108 108
Continue to review full report at Codecov.
|
Before this commit, all the node types were added to the same list of children nodes. Taking in consideration that only one Param and Any type of node could exist per node, two new node struct field were added to hold the references to those kind of nodes. This avoid the need to iterate through all the Static type nodes just to find one Param or Any type node. Those iterations could be performed multiple times in the same iteration of Router#Find. Removing the route comments of the Router benchmark tests. Updating the Router benchmarks tests to find the routes defined to each particular benchmark. Before, all the benchmarks tried to find only the GitHub API. Adding new router benchmarks to measure when the Router try to find routes that are not registered.
e280e4e
to
3a6100b
Compare
@pafuent I do not see too much risk in changed behaviour as our router tests are pretty helpful here. I'm missing our github action for benchmark comparison though. |
Sadly the GitHub action won't be fair in this case, because I changed the benchmarks on this PR. In this case you will need to trust on my benchmarks 😉 |
Please rebase to master. |
@lammel rebased |
Before this commit, all the node types were added to the same list of children nodes. Taking in consideration that only one Param and Any type of node could exist per node, two new node struct field were added to hold the references to those kind of nodes.
This avoid the need to iterate through all the Static type nodes just to find one Param or Any type node. Those iterations could be performed multiple times in the same iteration of Router#Find.
Removing the route comments of the Router benchmark tests.
Updating the Router benchmarks tests to find the routes defined to each particular benchmark. Before, all the benchmarks tried to find only the GitHub API.
Adding new router benchmarks to measure when the Router try to find routes that are not registered.
To be fair in the below benchmarks, I did the same modifications to the Router benchmarks on master. Also I performed 10 iterations per each benchmark, and compared the results with benchstat
benchstat bench_master.txt bench_new.txt