-
Notifications
You must be signed in to change notification settings - Fork 623
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
feat: optimize ID-driven selections #7933
Conversation
b54ecd1
to
28a2e93
Compare
Stores for ID-driven selections use a flattened structure, with tuples in the form of {unit, _vgsid_} sorted by _vgsid_. And inclusion testing now occurs via a new Vega expression function that binary searches through the store to determine a hit.
c1edade
to
5a6ca2d
Compare
16da17a
to
9551dc4
Compare
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 great. One question about the format. We store selection ids in sorted order so we can do a binary search. Why don't we store the ids in an object that would make it even easier to search and edit (e.g. some kind of set)?
That's a good idea but I don't think Vega has support for Sets/OrderedSets? I might've missed something though. But, in the event I haven't, I think it's a good idea for us to consider in the future or if/when we move selections down to the Vega level. |
Vega itself could definitely use Sets but they cannot be serialized as JSON. The solution for that is usually to represent a set as an object with values |
🚀 PR was released in |
Co-authored-by: GitHub Actions Bot <[email protected]>
With geo-interval (#6953) and lasso (#7932) selections on the horizon, there are going to be not only many more ID-driven selections than before, but these selections are likely going to capture many more IDs than before. As a result, this PR cherry picks selection ID-based optimizations from #6953 and improves on them in a more cross-cutting fashion.
In particular, stores for ID-driven selections use a flattened structure with tuples in the form of
{unit, _vgsid_}
sorted by_vgsid_
. And inclusion testing now occurs via a new Vega expression function that binary searches through the store to determine a hit.This PR relies on updates introduced in vega/vega#3397 (so runtime tests may not pass until that PR is merged and released).
📦 Published PR as canary version:
5.2.1--canary.7933.9a516dd.0
✨ Test out this PR locally via:
Version
Published prerelease version:
v5.3.0-next.2
Changelog
🎉 This release contains work from new contributors! 🎉
Thanks for all your work!
❤️ Alicia Schep (@AliciaSchep)
❤️ James Scott-Brown (@jamesscottbrown)
❤️ Cameron Yick (@hydrosquall)
❤️ Paul Rosenzweig (@paulrosenzweig)
❤️ Jon Mease (@jonmmease)
❤️ Sid Kapur (@sid-kap)
❤️ Robin Linacre (@RobinL)
❤️ Hao Chen (@ObservedObserver)
🚀 Enhancement
🐛 Bug Fix
next
Authors: 13