refactor(req): use URLSearchParams for parsing query params
#3565
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3565 +/- ##
==========================================
- Coverage 94.71% 94.68% -0.04%
==========================================
Files 157 157
Lines 9539 9480 -59
Branches 2819 2799 -20
==========================================
- Hits 9035 8976 -59
Misses 504 504 ☔ View full report in Codecov by Sentry. |
yusukebe
changed the title
URLSearchParams for parsing query paramsURLSearchParams for parsing query params
|
Hi @usualoma ! I want to hear your opinion! |
Another benchmarkWhen we do local benchmarking of query string parsing, I confirm that using diff --git a/benchmarks/query-param/src/bench.mts b/benchmarks/query-param/src/bench.mts
index e8be60e0..857d24c9 100644
--- a/benchmarks/query-param/src/bench.mts
+++ b/benchmarks/query-param/src/bench.mts
@@ -1,4 +1,5 @@
import { run, group, bench } from 'mitata'
+import { getQueryStrings } from '../../../src/utils/url'
import fastQuerystring from './fast-querystring.mts'
import hono from './hono.mts'
;[
@@ -36,6 +37,17 @@ import hono from './hono.mts'
group(JSON.stringify(data), () => {
bench('hono', () => hono(url, key))
bench('fastQuerystring', () => fastQuerystring(url, key))
+ bench('URLSearchParams', () => {
+ const params = new URLSearchParams(getQueryStrings(url))
+ if (key) {
+ return params.get(key)
+ }
+ const obj = {}
+ for (const [k, v] of params) {
+ obj[k] = v
+ }
+ return obj
+ })
})
})
|
Is it an app that works in a persistent environment?From the benchmark mentioned above, we can see that using |
|
As you mentioned in your comment, there is a trade-off between “small” and “fast” (and “small causes fast " complicates the problem), so I think it's a difficult point. When using The results of specific benchmarks will be worse, so we have to think about that. As an alternative, I think there is also the option of “using URLSearchParams when using the |
|
@yusukebe |
|
How about that code?: import { Hono } from 'hono/hono-base'
import { FastURLSearchParams } from 'hono/...'
const app = new Hono({
URLSearchParams: URLSearchParams
})and implementing using URLSearchParams when using the hono/tiny preset like @usualoma said is easier. |
|
@usualoma Thank you for the comments!
As you know, Hono has to support both "one-time" and "persistent" apps. For example, each popular environment has a different life cycle:
The following benchmarks for Cloudflare Workers are of interest. The smaller file size is faster. https://github.com/TigersWay/cloudflare-playground In that benchmark, the application receives one access every 60 seconds, which is not a lot. My guess is that once the access is handled, the application shuts down. And then it starts anew for the next access. In a real-world application, it will stay running and Hono will perform better because it handles more accesses than once every 60 seconds. However, many people make judgments based on benchmark results, so it is also important to be fast, even "one-time".
On the other hand, there is this way of thinking about it. For example, if you use a Hono app as a proxy server, as in the Issue #3518 , the code is short. However, even though you are not using
This is interesting! It is truly Either way, I believe this PR is effective for now. But there is no need to rush! |
|
In this case, the important point is that |
|
@yusukebe from what I see it makes sense to ship the fastest (default) with Hono preset and change it to Comparing Hono with Express is unfair, but one of the reasons why we've decided to use it instead of Fastify was its performance. I'm not asking to change its DNA or roots, but users might need to be aware of the trade-off. |
|
Thank you for the suggestion and your opinion!
You may saying right thing for us. We have to focus on speed. The only way to achieve both is by taking the idea to make |
|
I like the idea of having |
|
@yusukebe My motivation is that I want to migrate my current APIs to Hono :) I'm using a special query "syntax" which is supported by {
amount: {
lte: 300
}
}If there is another way to do this, I'm happy to ditch |
I see! But it's not good that the API will be changed depending on the implementation, such as Hono's current logic, |
This PR changes the implementation of parsing query params using
c.req.query(). This change affects performance and file size. In addition, it fixes the problem that can't parse invalid percent strings.File size
The logic to parse query params like
?name=honois written by ourown method. The method started with following:hono/src/utils/url.ts
Lines 213 to 217 in 3a59d86
This enables them to be parsed fast, but the logic and code are complicated.
With this PR, it uses
URLSearchParamsinstead of that logic. So, the code will decrease, and the application file size will be smaller. The below compares the current release version and this PR. The application is "Hello World" usinghono/tinypreset and minified withesbuild:The result is that this change decreases
642bytes. This seems to be a small change for the non-Hono user, but I think it's a big diff for us.The important point is that
honoshould not provide a lot of code for the user that does not use a lot of features. For example, we don't want to take much code to parse a query if the application does not usec.req.query(). So, usingURLSearchParamsbuilt-in API makes sense.Performance
Instead, performance will decrease. The following is the result:
The application code:
At a first grance, you may be impressed that this difference is enormous, but it's a slight, 4% slowdown. I think there's more advantage in using
URLSearchParasmfor making it short and easy to understand.Small vs Fast
In this place, you may think "small vs. fast "—which should we choose, small or fast? I think the answer is "both." We have to consider it case by case.
In this parsing query param matter, "small" is better. Being small has some time-saving effects because a small application can be faster in an environment where resources such as Cloudflare Workers are limited. So, in some cases, "small" makes "fast".
Parting invalid percent strings
This PR will fix the problem of paring invalid percent strings. Before this PR, it will throw the error if you want to do
c.req.query('q')for the following URL:In this PR, it can parse the query and you can get a
%h.Conclusion
This PR using
URLSearchParamsintroduce decrease of the performance to parse query params, but reducing the file size and making the code clean is more efficient. BUT, this is just my current thought. We can discuss it.