feat: Support reuse of nested router #393
Conversation
|
Hey @davidmartos96 👋
Not sure I follow, can you provide a bit more context regarding where excess Routers are dynamically created? In the meantime, I’ll run some benchmarks to gather some more quantitative data. Thanks again! 🙏 UPDATE I ran some preliminary benchmarks using apache benchmark and did not observe any significant performance differences between the latest version of Dart Frog (route with nested, dynamic middleware: Apache Benchmark Results on 2019 MacBook Pro (intel i9)Dart Frog Dev Dart Frog Prod Dart Frog Prod Compiled Shelf Shelf Compiled |
So taking the following snippet from the tests as an example: Handler createUserHandler(String user) {
final router = Router()
..get('/self', (RequestContext context) {
return Response(body: "I'm $user");
})
..get('/', (RequestContext context) {
return Response(body: '$user root');
});
return router;
}
final app = Router()
..mount('/users/<user>', (RequestContext context, String user) {
final handler = createUserHandler(user);
return handler(context);
})
..all('/<_|[^]*>', (RequestContext context) {
return Response(body: 'catch-all-handler');
});Each request to a resource in So where I see it could be problematic is a situation where you have a nested router ( Another point jonasf makes in the other thread is the following:
Having the creation of the router inside a handler prevents you to early-detect a possible error in a subroute right when you start the server. |
|
@felangel Hello! From my tests on my machine, with this PR, a dynamic nested router server performs 44% better. |
|
@felangel Any update on this? It would be great if you could take a look, as this could be beneficial to |
Sorry for the delay! I’m planning to revisit this sometime this week. Thanks again! |
|
Any updates? 😅 |
davidmartos96
changed the title
renancaraujo
requested review from
renancaraujo,
wolfenrain and
erickzanardo
as code owners
|
Hi @davidmartos96 , sorry for the delayed reply. We've not forgotten about this Pull Request. I will try looking at it, but it might take slightly longer than expected since I need to gather some context first. |
|
@alestiago Great to hear! The initial discussion around this was in the linked Original issues: Benchmarks: |
Status
PENDING DISCUSSION
Description
Hello! I've just noticed that supporting nested routers has been implemented, great news.
The implementation is very similar to one of my early iterations in dart-lang/shelf#288 . But according to jonasfj, creating a Router per each request could be resource intensive.
What I propose is a way to access the mounted params from within the RequestContext/Request so that a Router can be created before hand.
The implementation I did in this PR is the same solution as in the last iteration in the
shelf_routerPR, but there might be other solutions. I'm open to discuss it. I'm used to work with shelf and shelf_router as is, not with the dart_frog code generation, so there might be a solution that could fit nicely with it.Type of Change