-
Notifications
You must be signed in to change notification settings - Fork 14
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
Alternate implementation of route trie #6
Conversation
TCN-2192 Implement router that uses prefix tree
Needs to support nesting of variable definitions as well as wildcards and double-wildcards. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to get a benchmark, I think allocating maps for each variable capture is going to be expensive and it looks like it grows when adding new routes as you need to keep all variables alive (presumably small though).
From the current implementation I think theres a neat way to simplify the variable capture using indexes of segments. So the method
would have a field of an array of vars:
vars [][]struct{
fds []protoreflect.FieldDesc
start int
end int // -1 if `**` captures all remaining segments
}
Then if we convert a nested variable capture /v1/{a=part/{b}/*}
to part/*/*
with variables as {fds: "a", start: 1, end: 3}
, {fds: "b", start: 2, end: 2}
on matching a route to the method we can resolve variables by simply taking the index of segments from the path. Thoughts?
Ooh, that's very nice indeed! That gets rid of accumulation entirely in the hot path. |
… accumulate values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Hopefully this isn't too bad to understand. It supports the nesting of variable definitions by using a stack of paths when matching a route while descending through the trie.
This does not yet bother parsing a path template string into the AST model (the
routePath
struct). So the tests all use struct literals to createroutePath
instances.This implements both populating the trie and matching a route to an HTTP request.
Resolves TCN-2192