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

Matcher is linear and uses too many regex #3709

Open
iurisilvio opened this issue Jan 30, 2022 · 3 comments
Open

Matcher is linear and uses too many regex #3709

iurisilvio opened this issue Jan 30, 2022 · 3 comments

Comments

@iurisilvio
Copy link
Contributor

iurisilvio commented Jan 30, 2022

What problem does this feature solve?

The matcher implementation always use path-to-regex lib. It is too much for static paths, where it is possible to just compare strings.

Also, the implementatioin is O(N), if you have 100 routes and want to match the last one (that is static), you run 100 regex tests.
Regex is around ~10x slower than a simple string match and with a lookup table it is O(1).

for (let i = 0; i < pathList.length; i++) {
const path = pathList[i]
const record = pathMap[path]
if (matchRoute(record.regex, location.path, location.params)) {
return _createRoute(record, location, redirectedFrom)

What does the proposed API look like?

The API don't change, it is just optimization.

I did a draft PR #3707 to show the idea.

@posva
Copy link
Member

posva commented Feb 1, 2022

This is really nice but it's not worth the increased complexity and library size.

v4 (https://github.com/vuejs/router) uses a custom implementation so maybe you want to get a look in that one but the final size should be kept very similar.

The complexity O(n) is necessary to cater to some existing use cases and there are plans to provide a way to use a custom matcher with different strategies that would allow lighter and faster routers but less features

@iurisilvio
Copy link
Contributor Author

I understand the problem with increased complexity and library size.

I wrote the PR just as a proof of concept, but I think it can be simpler and smaller integrating to create-route-map. Do you consider the change if I do that?

The O(n) complexity is necessary in some cases, but not for static paths.

My nuxt app has ~100 routes and 70% are static, so I have a huge overhead and I think it is a common use case.

@iurisilvio
Copy link
Contributor Author

I did a simple static matcher implementation in #3713, matching with static routes before full route matching. I tried to do a minor change to improve matcher for static paths.

Another easy optimization would be to not test against static paths in current match loop, just marking some routes as static.

Would you consider this approach?

Also, I'll check the v4 implementation to optimize them if it is possible.

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