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

Add double colon handling (support for paths with colons) #176

Merged
merged 2 commits into from
Feb 9, 2021

Conversation

morigs
Copy link
Contributor

@morigs morigs commented Jan 13, 2021

Resolves #175

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.

Could you run the benchmark before and after the change and post the results?

@mcollina
Copy link
Collaborator

Good work!

@morigs
Copy link
Contributor Author

morigs commented Jan 14, 2021

Sure. My results are the following

Before

lookup static route x 46,229,112 ops/sec ±0.54% (591 runs sampled)
lookup dynamic route x 2,886,813 ops/sec ±0.43% (586 runs sampled)
lookup dynamic multi-parametric route x 1,614,329 ops/sec ±0.18% (589 runs sampled)
lookup dynamic multi-parametric route with regex x 1,115,574 ops/sec ±0.18% (587 runs sampled)
lookup long static route x 2,904,037 ops/sec ±0.20% (588 runs sampled)
lookup long dynamic route x 2,018,473 ops/sec ±0.19% (590 runs sampled)
lookup static versioned route x 6,800,830 ops/sec ±0.19% (586 runs sampled)
find static route x 30,635,586 ops/sec ±1.11% (573 runs sampled)
find dynamic route x 3,362,088 ops/sec ±0.15% (589 runs sampled)
find dynamic multi-parametric route x 1,899,912 ops/sec ±0.20% (589 runs sampled)
find dynamic multi-parametric route with regex x 1,245,699 ops/sec ±0.19% (590 runs sampled)
find long static route x 4,195,288 ops/sec ±0.20% (589 runs sampled)
find long dynamic route x 2,691,891 ops/sec ±0.18% (590 runs sampled)
find static versioned route x 8,167,230 ops/sec ±0.22% (583 runs sampled)
find long nested dynamic route x 1,043,135 ops/sec ±0.13% (590 runs sampled)
find long nested dynamic route with other method x 2,010,740 ops/sec ±0.16% (589 runs sampled)

After

lookup static route x 46,398,290 ops/sec ±0.53% (590 runs sampled)
lookup dynamic route x 2,882,147 ops/sec ±0.18% (589 runs sampled)
lookup dynamic multi-parametric route x 1,575,300 ops/sec ±0.17% (591 runs sampled)
lookup dynamic multi-parametric route with regex x 1,182,742 ops/sec ±0.16% (590 runs sampled)
lookup long static route x 2,946,382 ops/sec ±0.32% (588 runs sampled)
lookup long dynamic route x 2,027,316 ops/sec ±0.32% (589 runs sampled)
lookup static versioned route x 6,859,823 ops/sec ±0.21% (586 runs sampled)
find static route x 32,015,062 ops/sec ±0.59% (576 runs sampled)
find dynamic route x 3,404,869 ops/sec ±0.16% (590 runs sampled)
find dynamic multi-parametric route x 1,855,533 ops/sec ±0.14% (590 runs sampled)
find dynamic multi-parametric route with regex x 1,310,752 ops/sec ±0.15% (591 runs sampled)
find long static route x 4,085,360 ops/sec ±0.14% (590 runs sampled)
find long dynamic route x 2,673,217 ops/sec ±0.15% (591 runs sampled)
find static versioned route x 8,030,499 ops/sec ±0.20% (581 runs sampled)
find long nested dynamic route x 1,034,793 ops/sec ±0.18% (592 runs sampled)
find long nested dynamic route with other method x 1,981,866 ops/sec ±0.13% (592 runs sampled)

index.js Show resolved Hide resolved
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, this is ok as-is

Copy link
Owner

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

LGTM

@delvedor delvedor merged commit 191ad39 into delvedor:master Feb 9, 2021
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.

Allow routes with colons
3 participants