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

feat: Support reuse of nested router #393

Merged
merged 16 commits into from
Oct 31, 2023

Conversation

davidmartos96
Copy link
Contributor

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_router PR, 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

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@davidmartos96 davidmartos96 marked this pull request as draft October 19, 2022 08:17
@felangel
Copy link
Contributor

felangel commented Oct 19, 2022

Hey @davidmartos96 👋
Thanks so much for opening a PR and for taking the lead on helping support mounting dynamic routes! 💙

But according to jonasfj, creating a Router per each request could be resource intensive.

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: /users/<id>/<name>) and vanilla Shelf.

Apache Benchmark Results on 2019 MacBook Pro (intel i9)

Dart Frog Dev

Server Software:        
Server Hostname:        127.0.0.1
Server Port:            8080

Document Path:          /users/42/frog
Document Length:        20 bytes

Concurrency Level:      10
Time taken for tests:   6.220 seconds
Complete requests:      20000
Failed requests:        0
Keep-Alive requests:    0
Total transferred:      5500000 bytes
HTML transferred:       400000 bytes
Requests per second:    3215.62 [#/sec] (mean)
Time per request:       3.110 [ms] (mean)
Time per request:       0.311 [ms] (mean, across all concurrent requests)
Transfer rate:          863.57 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0  19.4      0    1582
Processing:     0    3  34.0      2    1583
Waiting:        0    3  34.0      2    1583
Total:          1    3  39.1      2    1583

Percentage of the requests served within a certain time (ms)
  50%      2
  66%      2
  75%      2
  80%      2
  90%      2
  95%      2
  98%      3
  99%      3
 100%   1583 (longest request)

Dart Frog Prod

Server Software:        
Server Hostname:        127.0.0.1
Server Port:            8080

Document Path:          /users/42/frog
Document Length:        20 bytes

Concurrency Level:      10
Time taken for tests:   6.559 seconds
Complete requests:      20000
Failed requests:        0
Keep-Alive requests:    0
Total transferred:      5500000 bytes
HTML transferred:       400000 bytes
Requests per second:    3049.05 [#/sec] (mean)
Time per request:       3.280 [ms] (mean)
Time per request:       0.328 [ms] (mean, across all concurrent requests)
Transfer rate:          818.84 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   1.9      0     266
Processing:     0    3  47.2      2    2089
Waiting:        0    3  44.9      2    2089
Total:          1    3  47.3      2    2089

Percentage of the requests served within a certain time (ms)
  50%      2
  66%      2
  75%      2
  80%      2
  90%      2
  95%      3
  98%      3
  99%      3
 100%   2089 (longest request)

Dart Frog Prod Compiled

Server Software:        
Server Hostname:        127.0.0.1
Server Port:            8080

Document Path:          /users/42/frog
Document Length:        20 bytes

Concurrency Level:      10
Time taken for tests:   6.541 seconds
Complete requests:      20000
Failed requests:        0
Keep-Alive requests:    0
Total transferred:      5500000 bytes
HTML transferred:       400000 bytes
Requests per second:    3057.84 [#/sec] (mean)
Time per request:       3.270 [ms] (mean)
Time per request:       0.327 [ms] (mean, across all concurrent requests)
Transfer rate:          821.20 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       1
Processing:     0    3  48.5      2    2122
Waiting:        0    3  48.5      2    2122
Total:          1    3  48.5      2    2122

Percentage of the requests served within a certain time (ms)
  50%      2
  66%      2
  75%      2
  80%      2
  90%      2
  95%      2
  98%      3
  99%      3
 100%   2122 (longest request)

Shelf

Server Software:        
Server Hostname:        127.0.0.1
Server Port:            8080

Document Path:          /users/42/frog
Document Length:        20 bytes

Concurrency Level:      10
Time taken for tests:   6.745 seconds
Complete requests:      20000
Failed requests:        0
Keep-Alive requests:    0
Total transferred:      5420000 bytes
HTML transferred:       400000 bytes
Requests per second:    2965.34 [#/sec] (mean)
Time per request:       3.372 [ms] (mean)
Time per request:       0.337 [ms] (mean, across all concurrent requests)
Transfer rate:          784.77 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   3.5      0     492
Processing:     0    3  63.8      1    2792
Waiting:        0    3  60.7      1    2792
Total:          1    3  63.9      1    2792

Percentage of the requests served within a certain time (ms)
  50%      1
  66%      2
  75%      2
  80%      2
  90%      2
  95%      2
  98%      2
  99%      2
 100%   2792 (longest request)

Shelf Compiled

Server Software:        
Server Hostname:        127.0.0.1
Server Port:            8080

Document Path:          /users/42/frog
Document Length:        20 bytes

Concurrency Level:      10
Time taken for tests:   6.730 seconds
Complete requests:      20000
Failed requests:        0
Keep-Alive requests:    0
Total transferred:      5420000 bytes
HTML transferred:       400000 bytes
Requests per second:    2971.83 [#/sec] (mean)
Time per request:       3.365 [ms] (mean)
Time per request:       0.336 [ms] (mean, across all concurrent requests)
Transfer rate:          786.49 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       1
Processing:     0    3  73.8      1    3267
Waiting:        0    3  70.1      1    3267
Total:          1    3  73.8      1    3267

Percentage of the requests served within a certain time (ms)
  50%      1
  66%      1
  75%      2
  80%      2
  90%      2
  95%      2
  98%      2
  99%      2
 100%   3267 (longest request)

@davidmartos96
Copy link
Contributor Author

@felangel

Not sure I follow, can you provide a bit more context regarding where excess Routers are dynamically created?

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 /users/<something> will be creating a different router, that as a result each of the nested router entries (get, put, ...) will be running a quite complex regular expression.

static final RegExp _parser = RegExp(r'([^<]*)(?:<([^>|]+)(?:\|([^>]*))?>)?');

for (final m in _parser.allMatches(route)) {

So where I see it could be problematic is a situation where you have a nested router (/users/<user>/) and the user route has a quite some resources defined (10-20). Then for each request to any of those resources, 10-20 regexps will be processed.
But I haven't benchmarked this situation. If you have the benchmark code from above and the tool you used I would be happy to give it a try.

Another point jonasf makes in the other thread is the following:

Moreover it's nice that all routes are evaluated early on, so that issues in patterns throw at the server startup time.

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.

@davidmartos96
Copy link
Contributor Author

@felangel Hello!
I had some time to run some benchmarks (https://github.com/davidmartos96/dart_frog_dynamic_router_benchmark)

From my tests on my machine, with this PR, a dynamic nested router server performs 44% better.
The benchmark code adds 20 subroutes to the nested router, which is where the bottleneck could appear. #393 (comment)

@davidmartos96
Copy link
Contributor Author

@felangel Any update on this? It would be great if you could take a look, as this could be beneficial to dart_frog in terms of performance. +40% difference could be noticeable at big scale.

@felangel
Copy link
Contributor

@felangel Any update on this? It would be great if you could take a look, as this could be beneficial to dart_frog in terms of performance. +40% difference could be noticeable at big scale.

Sorry for the delay! I’m planning to revisit this sometime this week. Thanks again!

@davidmartos96
Copy link
Contributor Author

Any updates? 😅

@davidmartos96 davidmartos96 changed the title Support reuse of nested router feat: Support reuse of nested router Jun 17, 2023
@renancaraujo renancaraujo marked this pull request as ready for review July 20, 2023 18:14
@alestiago alestiago added the feature A new feature or request label Aug 24, 2023
@alestiago
Copy link
Contributor

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.

@davidmartos96
Copy link
Contributor Author

@alestiago Great to hear! The initial discussion around this was in the linked shelf issue.
The benchmark of the optimization of router parameters with this PR should be working with today's dart_frog code as well I believe.

Original issues:
dart-lang/shelf#250
dart-lang/shelf#288

Benchmarks:
https://github.com/davidmartos96/dart_frog_dynamic_router_benchmark

@tomarra tomarra merged commit 6e87c6e into VeryGoodOpenSource:main Oct 31, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature or request
Projects
Development

Successfully merging this pull request may close these issues.

7 participants