-
Notifications
You must be signed in to change notification settings - Fork 305
perf(sync-service): Use ETS tables instead of maps in Filter to reduce GC pressure #3547
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3547 +/- ##
=======================================
Coverage 75.21% 75.21%
=======================================
Files 51 51
Lines 2744 2744
Branches 409 405 -4
=======================================
Hits 2064 2064
Misses 678 678
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
benchmark this |
magnetised
left a comment
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.
Great stuff!
I'm trusting the tests to validate the functionality -- haven't reviewed the algorithm itself. Can we cull the value-less comments and docs though pls.
packages/sync-service/lib/electric/shapes/filter/indexes/equality_index.ex
Outdated
Show resolved
Hide resolved
packages/sync-service/lib/electric/shapes/filter/indexes/inclusion_index.ex
Outdated
Show resolved
Hide resolved
packages/sync-service/lib/electric/shapes/filter/where_condition.ex
Outdated
Show resolved
Hide resolved
|
This looks pretty good but a bit hard to review - in the meantime might be worth running a benchmark on it as well? |
|
benchmark this |
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
7fbef29 to
9b37411
Compare
1ec78ae to
97161fa
Compare
msfstef
left a comment
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.
This looks sane!
packages/sync-service/lib/electric/shapes/filter/where_condition.ex
Outdated
Show resolved
Hide resolved
|
Just reading through the PR body again. It's probably worth calling out that the micro benchmarks are slower but when running, it's faster due to no/less gc pauses. |
|
This PR has been released! 🚀 The following packages include changes from this PR: Thanks for contributing to Electric! |
…e comments The release notification comments were missing package data because backticks in the body text (e.g., `@electric-sql/client@1.2.8`) were being interpreted by the shell as command substitution. This caused the package names to be replaced with empty strings. Fix by using gh's --body-file - flag to pass the body via stdin, avoiding shell interpretation entirely. Fixes comments like #3547 (comment)
The release notification comments were missing package data because backticks in the body text (e.g., `@electric-sql/client@1.2.8`) were being interpreted by the shell as command substitution. This caused the package names to be replaced with empty strings. Fix by using gh's --body-file - flag to pass the body via stdin, avoiding shell interpretation entirely. Fixes comments like #3547 (comment) Co-authored-by: Claude <noreply@anthropic.com>
Summary
Migrates the Filter module and its namespace (WhereCondition, EqualityIndex, InclusionIndex) from
Elixir maps to ETS tables to reduce garbage collection pressure with large numbers of shapes
(200K+).
Problem
With 200K shapes being added and removed frequently, the map-based implementation causes large GC
delays due to copying immutable map data structures on every add/remove operation.
Solution
Store filter data in ETS tables (outside the process heap) instead of nested Elixir maps:
incl_index_table
Performance Results (200K shapes)
The ETS implementation meets the <100µs target for affected_shapes with 200K equality-indexed
shapes.
Key Changes