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

Parsing Url is one of the most expensive things that async-h1 does #177

Open
jbr opened this issue Jan 24, 2021 · 3 comments
Open

Parsing Url is one of the most expensive things that async-h1 does #177

jbr opened this issue Jan 24, 2021 · 3 comments

Comments

@jbr
Copy link
Member

jbr commented Jan 24, 2021

In most cases, we parse a url from host and path and then later on only use the path for routing. Holding two &str's on Request and only generating a url if/when it's required would provide performance gains.

@yoshuawuyts
Copy link
Member

In most cases, we parse a url from host and path and then later on only use the path for routing.

When routing on the path we do need to parse the URL in order to filter out the query string, and normalize URL escaping. We can probably skip parsing the URL in async-h1, but I'm not sure to which degree it's feasible for e.g. tide.

If we think we'll have to parse URLs for tide anyway, is it still worth adding extra logic to async-h1 to not always have to parse them?

@jbr
Copy link
Member Author

jbr commented Jan 25, 2021

It might be worth checking if percent_encoding::percent_decode_str(path.split("?").next()) is faster than Url decode. However, this would have to happen at the same time as http-types v3, since there is currently no way to construct a Request without a full Url.

@jbr
Copy link
Member Author

jbr commented Jan 25, 2021

I also think it's worth asking why we need to parse urls for tide. Why does the host and path have to be represented as a Url specifically?

Alternatively, we could try to add a constructor for Url, or fork Url to add a constructor, since currently we are concatenating strings and then parsing them in order to construct a Url from scheme, host and path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants