honojs / hono

Web framework built on Web Standards
https://hono.dev
MIT License
20.59k stars 601 forks source link

perf(trie-router): optimize and remove unnecessary processes #3647

Closed EdamAme-x closed 2 weeks ago

EdamAme-x commented 3 weeks ago
  1. The following process is not necessary and has been removed.

https://github.com/honojs/hono/pull/3647/files#diff-bf1549ba9405f87c376d2d338883f54bfeb6a91f40b28e62dd2ce5e9d4edd4e5L66-L69

  1. Since it will be minified, I changed the name to be easy to understand.

Related: https://github.com/honojs/hono/pull/3596

https://github.com/honojs/hono/pull/3647/files#diff-bf1549ba9405f87c376d2d338883f54bfeb6a91f40b28e62dd2ce5e9d4edd4e5L85

  1. Since only the type number is used for the key, an empty object should be created instead of Object.create(null). It is faster and smaller.

Node and Bun (8x~ faster)

runtime: node 20.18.0 (x64-linux)

benchmark              avg (min … max) p75   p99    (min … top 1%)
-------------------------------------- -------------------------------
= Object.create(null)    16.67 ns/iter  15.13 ns  █                   
                 (13.54 ns … 96.35 ns)  42.71 ns ▇█▂▁▂▂▁▁▂▁▁▁▁▁▁▁▁▁▁▁▁
= {}                      2.41 ns/iter   2.22 ns  █                   
                  (1.84 ns … 95.40 ns)   8.21 ns ▁█▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁
runtime: deno 2.0.5 (x86_64-unknown-linux-gnu)

benchmark              avg (min … max) p75   p99    (min … top 1%)
-------------------------------------- -------------------------------
= Object.create(null)    16.64 ns/iter  14.93 ns  █                   
                (12.03 ns … 137.93 ns)  33.21 ns ▁██▃▁▁▁▁▁▁▁▁▁▁▅▂▁▁▁▁▁
= {}                      2.70 ns/iter   2.19 ns █                    
                  (2.03 ns … 62.84 ns)  16.17 ns █▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁

In Bun, the speed of both is almost the same.

https://github.com/honojs/hono/pull/3647/files#diff-bf1549ba9405f87c376d2d338883f54bfeb6a91f40b28e62dd2ce5e9d4edd4e5L95

4. Multiple index accesses are eliminated and the size is reduced.

https://github.com/honojs/hono/pull/3647/files#diff-bf1549ba9405f87c376d2d338883f54bfeb6a91f40b28e62dd2ce5e9d4edd4e5R130

  1. Remove as it is not a necessary check

https://github.com/honojs/hono/pull/3647/files#diff-bf1549ba9405f87c376d2d338883f54bfeb6a91f40b28e62dd2ce5e9d4edd4e5L178

The author should do the following, if applicable

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.69%. Comparing base (8a8a576) to head (9028f2f). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3647 +/- ## ========================================== + Coverage 91.68% 91.69% +0.01% ========================================== Files 159 159 Lines 10135 10138 +3 Branches 2878 2880 +2 ========================================== + Hits 9292 9296 +4 + Misses 841 840 -1 Partials 2 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

yusukebe commented 2 weeks ago

Hi @EdamAme-x !

It looks awesome! I'd like to know how much this change affects performance. Did you benchmark it? And how much did you reduce the bundle size?

PS.

To benchmark the req/res performance, you can follow the method used in this repo: https://github.com/SaltyAom/bun-http-framework-benchmarkhttps://github.com/SaltyAom/bun-http-framework-benchmark

EdamAme-x commented 2 weeks ago

Report

Revert (4): The bundle size is smaller, but we got strange benchmark results that one index access is slower than multiple index accesses, so I reverted.

runtime: deno 2.0.5 (x86_64-pc-windows-msvc)

benchmark              avg (min … max) p75   p99    (min … top 1%)
-------------------------------------- -------------------------------
Once                    193.31 ps/iter 195.31 ps ▇  █
                (170.90 ps … 70.87 ns) 341.80 ps █▁▁█▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁
Multi                   136.00 ps/iter 146.48 ps     █
                 (97.66 ps … 22.39 ns) 219.73 ps ▁▁▁▁█▁▁▁▄▁▁▁▂▁▁▁▂▁▁▁▁

Bundle Size

19.34kb => 19.28kb (1kb = 1024b)

gitpod@honojs-hono-0oouxesc90f:/workspace/hono/sandbox$ ls compare -l
total 40
-rw-r--r-- 1 gitpod gitpod 19288 Nov  9 07:09 after.js
-rw-r--r-- 1 gitpod gitpod 19344 Nov  9 07:11 before.js

Benchmark

In Deno

There was not much change. (+0.3%)

|  Framework       | Runtime | Average | Ping       | Query      | Body       |
| ---------------- | ------- | ------- | ---------- | ---------- | ---------- |
| after | deno | 41,225.407 | 50,386.78 | 38,903.5 | 34,385.94 |
| before | deno | 41,079.073 | 50,923.38 | 39,116.56 | 33,197.28 |
yusukebe commented 2 weeks ago

@EdamAme-x

Have you ever tried to benchmark frameworks on Bun, not only Deno?

EdamAme-x commented 2 weeks ago

I'll benchmark with several approaches.

EdamAme-x commented 2 weeks ago

I measured the performance of router itself using node, bun, and mitata, but did not see any significant performance improvement that we could see.

In Deno sample

clk: ~3.03 GHz
cpu: AMD EPYC 7B13
runtime: deno 2.0.5 (x86_64-unknown-linux-gnu)

benchmark              avg (min … max) p75   p99    (min … top 1%)
-------------------------------------- -------------------------------
Before Trie               3.55 µs/iter   3.57 µs   ▅ █▅██   ▅         
                   (3.47 µs … 3.70 µs)   3.68 µs ▄▇█▄████▄▄▄█▁▄▇▁▁▁▄▁▁
After Trie                3.51 µs/iter   3.52 µs   ▄▆▂█▂▄             
                   (3.45 µs … 3.71 µs)   3.67 µs ▇▅██████▃▃▁▁▁▃▁▁▁▁▁▁▁
yusukebe commented 2 weeks ago

@EdamAme-x

Thanks. Anyway, this is a good improvement. I'll merge this now.