Performance regression from 2.1 to 2.2
I found a large performance regression from hanami-router 2.1 to 2.2.
I've compiled figures and graphs below using r10k. Special thanks to @jeremyevans for building r10k; it's a great tool.
I did these using hanami-api, which is a thin layer on top of hanami-router. Testing the full hanami gem has similar results, but hanami-api is closer to hanami-router. You can also test hanami-router yourself if you'd like (see below).
RPS
| Number of routes | hanami-router 2.1 | hanami-router 2.2 | Percent decrease of RPS |
|---|---|---|---|
| 10 | 162282 RPS | 150325 RPS | 7.3% |
| 100 | 125929 RPS | 116285 RPS | 7.7% |
| 1000 | 93324 RPS | 45422 RPS | 51% |
| 10000 | 74545 RPS | 5480 RPS | 93% |
Runtime with startup
| Number of routes | hanami-router 2.1 | hanami-router 2.2 | Percent slower |
|---|---|---|---|
| 10 | 0.218s | 0.227s | 4.1% |
| 100 | 0.251s | 0.266s | 6.0% |
| 1000 | 0.357s | 0.559s | 56% |
| 10000 | 1.186s | 4.098s | 245% |
Graphs
All generated from M1 Macbook Pro w/ 16GB of RAM, with Ruby 3.3.6 (not 3.4), and some normal apps in the background (not isolated at all)
Reproduction steps
gem uninstall hanami-routerto get a clean slategem install hanami-api gruff(gruff is for generating graphs)gem install hanami-router --version 2.1.0gem list hanami-routershould only show 2.1.0- Clone https://github.com/jeremyevans/r10k
- From that folder,
rake bench R10K_APPS="hanami-api" - Add a suffix to every file in
data/, so they don't get overwritten when we run it again. gem install hanami-router --version 2.2.0rake bench R10K_APPS="hanami-api"again- Open the new data/*.csv files and copy over the rows from the suffixed files, and update each row to reflect which version of hanami-router was used for each.
- Finally, run
rake graphsto generate graphs from those CSV's
Here are the results I got. Requests per scond: rps.csv Runtime including startup: runtime_with_startup.csv
Also here's a memory file I did too, but the figures are almost the same so it doesn't look like a memory issue: memory.csv
Suspected source of regression
I suspect it has to do with changing the trie implementation in #273.
@kyleplump @dcr8898 would either or both of you be willing to look into this? The apps generated in apps/ may be useful, especially hanami-api_4_10.rb since that's where things get really bad.
I also wrote a builder that uses hanami-router directly. It's slightly faster than hanami-api, as one would expect, but not drastically different. https://github.com/cllns/r10k/blob/add-hanami-router/builders/hanami-router.rb
@cllns that "345% slower" metric made me physically recoil 😐
i suspect it might be because of the fact that 2.2 delays creating Mustermann matchers - leading to potentially some significant overhead that didn't exist before. i'd be happy to reproduce and experiment!
@kyleplump Sorry about that! 10,000 routes is a situation we don't need to optimize for but it helps illustrate the problem and hopefully make it faster to find the problem to fix it. Also, I messed up the "percent slower" computation! Also I messed up the calculation, so it's "only" 245% slower. Small solace I guess 😅
Thanks for looking into it!
Hi, Sean! Thank you for bringing this to light! It's definitely from our refactor of the trie implementation. In fact, I wouldn't call this a regression. It's a straight degradation. 🤣😭😰
I don't think the problem is the Mustermann matchers. This implementation creates far fewer than before, and doesn't create one until there is a potential matching route to match against.
I believe the algorithm is sound, but the implementation is inefficient. I would love to work on it further with Kyle, you, and/or anyone else with performance experience. I have none and was not sure how to test that at the time.
Maybe we can arrange a time together? Mastodon might be the best way to coordinate.
Thanks again! Happy New Year! 🥳
Damian
On Tue, Dec 31, 2024, 3:36 PM Sean Collins @.***> wrote:
@kyleplump https://github.com/kyleplump Sorry about that! 10,000 routes is a situation we don't need to optimize for but it helps illustrate the problem and hopefully make it faster to find the problem to fix it.
Thanks for looking into it!
— Reply to this email directly, view it on GitHub https://github.com/hanami/router/issues/278#issuecomment-2566700991, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC7IFGDAJKNM2D54OTI7I3T2IL55HAVCNFSM6AAAAABUNTN45KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRWG4YDAOJZGE . You are receiving this because you were mentioned.Message ID: @.***>
hi @dcr8898, @cllns
opened an issue related to this: #279 - would love to hear what you guys think about it!