Store projections in a tree of patterns #188
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #188 +/- ##
==========================================
+ Coverage 91.01% 91.14% +0.13%
==========================================
Files 20 21 +1
Lines 3494 3535 +41
==========================================
+ Hits 3180 3222 +42
+ Misses 314 313 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
the-mikedavis
force-pushed
the
pattern-tree
branch
from
2105b93
to
56d1d1a
Compare
| %% @see fold_fun(). | ||
| %% @see fold_acc(). | ||
|
|
||
| fold(PatternTree, Tree, Path, FoldFun, Acc) -> |
There was a problem hiding this comment.
I'm split on the naming of this function. I like fold because we have the FoldFun and Acc arguments. It could also be called find_matching_patterns though to mimic khepri_tree:find_matching_nodes/5 which also has a FoldFun and Acc.
There was a problem hiding this comment.
Probably khepri_tree:find_matching_nodes/5 should be renamed fold/5. What it does changed a lot since its creation and the name remained the same.
There was a problem hiding this comment.
Do you think it's worth renaming khepri_tree:find_matching_nodes/5 now? I think fold/5 would fit better for that too
There was a problem hiding this comment.
Yes, now that's it's a separate module, the name you suggest is better.
There was a problem hiding this comment.
Ok! I will create a separate PR with the rename 👍
| %% @see fold_fun(). | ||
| %% @see fold_acc(). | ||
|
|
||
| fold(PatternTree, Tree, Path, FoldFun, Acc) -> |
There was a problem hiding this comment.
Probably khepri_tree:find_matching_nodes/5 should be renamed fold/5. What it does changed a lot since its creation and the name remained the same.
The main tree structure used to store data in the Khepri store is a tree where each edge is a path component and that tree can look up all matching paths for a given pattern. This change introduces a new tree structure that instead uses pattern components as edges and can be used to look up all matching patterns for a given path. This replaces the data structure used to hold projections in the machine's state. The prior structure was a mapping from projections to path patterns. Every change to the store was checked against all patterns for registered projections using 'khepri_tree:does_path_match/3'. The existing approach is fast in practice but it duplicates some work: pattern components shared between multiple patterns would all be re-compiled and re-checked for each projection and for every change to the store. The new pattern tree eliminates the duplicate work and saves a small but noticeable amount of time when a store uses many projections and sees a large number of changes.
the-mikedavis
force-pushed
the
pattern-tree
branch
from
56d1d1a
to
5f5f619
Compare
Depends on #187The main tree structure used to store data in the Khepri store is a tree where each edge is a path component and that tree can look up all matching paths for a given pattern. This change introduces a new tree structure that instead uses pattern components as edges and can be used to look up all matching patterns for a given path.
This replaces the data structure used to hold projections in the machine's state. The prior structure was a mapping from projections to path patterns. Every change to the store was checked against all patterns for registered projections using
khepri_tree:does_path_match/3. The existing approach is fast in practice but it duplicates some work: pattern components shared between multiple patterns would all be re-compiled and re-checked for each projection and for every change to the store. The new pattern tree eliminates the duplicate work and saves a small but noticeable amount of time when a store uses many projections and sees a large number of changes.In particular I tested this against a
rabbitmqctl import_definitionsof 1 million topic bindings against thekhepribranch in the server's repository. The time with this change is now an improvement over mnesia:We can compare the flamegraphs recorded during these tests...
main:This branch:
One of the largest spans in the flamegraph on
mainis around thekhepri_machine:create_projection_side_effects/3function which spends much of its time inkhepri_tree:does_path_match/3,4andkhepri_condition:compile/1under that. This branch eliminates the duplicatekhepri_condition:compile/1calls and spends less time checking whether patterns match.