⚡️ Ensure faster start-up when using many nested routers#10589
⚡️ Ensure faster start-up when using many nested routers#10589jvllmr wants to merge 49 commits intofastapi:masterfrom
Conversation
|
I refactored the pull-request according to the latest changes. In the meantime the performance improved already (probably because of changes in Pydantic). In my benchmark the median improvement was around 8 to 10x. My benchmark is very simple and there might be more improvement in real world applications. The tests are failing as of now. I will look into it as soon as I can. |
|
Hi @jvllmr!
There was an unrelated issue with our CI that is now fixed, so you shouldn't see so many failing tests anymore. The only test still failing here is the one measuring "coverage" - this basically ensures that all code paths in the code base are tested in our test suite. So, to fix it, it would be good to add a test with this PR that checks the correct behaviour of the new functionality introduced here 🚀 Specifically, when I look at the detailed coverage report, it's the |
|
Thanks for the hint. I will see if I can come up with some good tests over the weekend. |
|
📝 Docs preview for commit 1689b18 at: https://d14f0dc6.fastapitiangolo.pages.dev Modified Pages |
|
I updated the PR with tests for all the new logic I introduced, but I don't know why 99.0% coverage are still shown here. Anyway, I added more FastAPI features to my benchmark and the improvement became even more significant: |
|
I actually found the offending lines. For some reason the summary at the bottom showed 100%, but I found the lines immediately when checking "Hide covered". All green now ✅ 🏁 |
remove useless checks in test test defer_init flag on route add route to test app fix coverage again
|
I dropped the unintentional commits just to be sure |
|
Thanks! We've got a bit of a reviewing queue at the moment, but we've added this one and will get back to you once we've been able to review it in more detail 🙏 |
|
This pull request has a merge conflict that needs to be resolved. |
Signed-off-by: Jan Vollmer <[email protected]>
Signed-off-by: Jan Vollmer <[email protected]>
100f6a0 to
46932ae
Compare
Signed-off-by: Jan Vollmer <[email protected]>
Signed-off-by: Jan Vollmer <[email protected]>
|
This pull request has a merge conflict that needs to be resolved. |
|
This pull request has a merge conflict that needs to be resolved. |
Signed-off-by: Jan Vollmer <[email protected]>
Signed-off-by: Jan Vollmer <[email protected]>
|
This pull request has a merge conflict that needs to be resolved. |
Signed-off-by: Jan Vollmer <[email protected]>
|
This pull request has a merge conflict that needs to be resolved. |
Signed-off-by: Jan Vollmer <[email protected]>
Signed-off-by: Jan Vollmer <[email protected]>
First of all: Thanks a lot for your on this project! I never thought that Python Web Development (especially for back-ends) could be this much more fun.
Now to the main part: In my FastAPI project I have a lot of nested routers. Because the amount of nested routers I soon reached a FastAPI start-up time of 30 seconds or more. This hurt my developer experience a lot and I soon began digging in to why FastAPI takes so long to load.
After some research I found out that each router calculates all relevant infos on initialization. A router, which includes another router, will re-calculate these values without re-using information from the router it includes. When having a flat project structure this is not a problem.
So the solution was simple: Defer the calculation of values until they are actually needed. By doing that the start-up time was shortened significantly.
One caveat remains: Deferring the calculation results to Exceptions not being raised at start-up and they are only raised at the first call of a route. So they might not be caught if a route is not tested at least once.
This pull request is my proposal to integrate this performance optimization into FastAPI without breaking user space and having to modify tests. I mitigated the Exception's issue by introducing an argument to the APIRouter and APIRoute which decides if values are deferred or not and never deferring values in the top-level (app-integrated) router.
I'm sure there might be a better way. So let me hear if you have suggestions for improvement!
You can find my research and testing environment for this here: https://github.com/jvllmr/fastapi-deferred-init