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

change path for short static route bench #225

Conversation

ivan-tymoshenko
Copy link
Collaborator

When I tried to optimize the find method, I was confused. I made changes that were supposed to speed up the algorithm, but they only made it slower for short static routes. It turns out that / route is a super special case, cause you get the results immediately from the root node without using the searching algorithm. If you try to search the path that contains only one more symbol /a, you will get at least twice a worse result. IMHO, when we test the find method we want to see results for some average, but not for the extreme case.

@@ -14,7 +14,7 @@ const suite = Benchmark.Suite()
const FindMyWay = require('./')

const findMyWay = new FindMyWay()
findMyWay.on('GET', '/', () => true)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please retain this route

bench.js Show resolved Hide resolved
@ivan-tymoshenko ivan-tymoshenko force-pushed the change-path-for-short-static-route-bench branch from 09c60db to 9e12efc Compare December 27, 2021 11:25
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit a2aefaa into delvedor:main Dec 27, 2021
@ivan-tymoshenko ivan-tymoshenko deleted the change-path-for-short-static-route-bench branch April 17, 2022 14:15
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

Successfully merging this pull request may close these issues.

2 participants