Skip to content

Redesign #18

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

Closed
fundon opened this issue Jun 18, 2022 · 13 comments
Closed

Redesign #18

fundon opened this issue Jun 18, 2022 · 13 comments

Comments

@fundon
Copy link
Member

fundon commented Jun 18, 2022

Named parameters

Named parameters are defined by prefixing a colon to the parameter name (:name), pattern is ([^\/#\?]+).

/heros/:name

path match params
/heros/io YES name=io
/heros/morphling YES name=morphling
/heros/123 YES name=123
/heros/io/123 NO
/heros/ NO
/foo `/foo/bar`
/foo/bar `/foo/bar`
/:name `/rust`
/:name.:ext `/users.json`
/:year.:month.:day/:name.:ext `/2022.12.12/users.json`
/:controller::action   `/users:run`
/*any `/any`
/prefix:param::postfix
@fundon
Copy link
Member Author

fundon commented Jun 22, 2022

/
├── *any
├── :controller::action
├── :name
├── :name.:ext
├── :year.:month.:day
│  ├── :name
│  └── :name.:ext
│  
└── prefix
│         └── :param
│                   └── :
│                       └── :postfix
└── foo
   └── bar

@adriangb
Copy link
Contributor

adriangb commented Jul 8, 2022

I'm not sure what these examples map to, but one thing that would be nice to have is the ability to match before/after params: /prefix:param::postfix or something like this.

@fundon
Copy link
Member Author

fundon commented Jul 8, 2022

@adriangb https://cloud.google.com/apis/design/custom_methods Google API Design.
Yes, good example, I am collecting scenes.

@adriangb
Copy link
Contributor

adriangb commented Jul 8, 2022

Ah yes never got into that stuff, but I've heard of it/seen it going around before.

I thought this was an interesting idea for benchmarking / minimum necessary features: https://github.com/richardolsson/falcon-routing-survey. Looking at the APIs for major companies makes sense, it's a realistic representation of what is needed routing-wise for a large well designed API.

@fundon
Copy link
Member Author

fundon commented Aug 1, 2022

@fundon
Copy link
Member Author

fundon commented Aug 22, 2022

@fundon
Copy link
Member Author

fundon commented Sep 22, 2022

hi @Txuritan, I have refactored and the routing syntax has been enhanced and the performance has been greatly improved. If you have time, please review the code and see if there are any optimization points. Thanks

@fundon
Copy link
Member Author

fundon commented Sep 22, 2022

Landed in v0.5.0. Next supports regex.

@fundon fundon closed this as completed Sep 22, 2022
@Txuritan
Copy link
Collaborator

I took a look using my normal profiling tools and I don't see any issues other than this binary_search_by and this from_str. Benchmarks only showed it being ~10us behind matchit (when using mimalloc).

I didn't see any issues in any of the finds (though I still need to look at inserting), though one small change I did try was replacing all the double pushs with a single on using Ranges to remove this chunk, this didn't improve anything other than readability in the end.

One optimization I do plan on trying is to use slab allocation to keep all nodes in the same memory space though I'm not sure if this will help in the end. I may also try to remove the required lifetime and play around this char based pats instead of u8s. I'll report back if any of these are successful.

@adriangb
Copy link
Contributor

For what it's worth adding the lifetimes makes it really hard to wrap this with PyO3 because there's nothing to own the PathTree struct (you can't hand over structs with lifetimes to Python)

@Txuritan
Copy link
Collaborator

That was my thought as well, I'm hoping I can remove it without causing a major slowdown anywhere.

@Txuritan
Copy link
Collaborator

It looks like removing the lifetime will cause an increase but it is still faster than it was before, though not by much.

As a comparison I went and ran the benchmark with 0.4.0 included:

matchit path-tree (w/o lifetime) path-tree 0.4.0
insert 58μs 78-81μs 81μs 98-103μs
find 30-31μs 40-41μs 41μs 47-51μs

Note: Same as before, the benchmark was ran with mimalloc with the security features disabled.

@adriangb
Copy link
Contributor

That seems like a super minor (if even statistically significant) regression. It would allow me to continue wrapping it with Python, I don’t think that’s possible with the lifetimes in there.

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

3 participants