Skip to content
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

Merged
merged 5 commits into from Apr 7, 2022
Merged

feat: optimize ID-driven selections #7933

merged 5 commits into from Apr 7, 2022

Conversation

arvind
Copy link
Member

@arvind arvind commented Jan 20, 2022

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:

npm install vega-lite@5.2.1--canary.7933.9a516dd.0
# or 
yarn add vega-lite@5.2.1--canary.7933.9a516dd.0

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

⚠️ Pushed to next

Authors: 13

@arvind arvind force-pushed the as/optimizedSelectionIDs branch 2 times, most recently from b54ecd1 to 28a2e93 Compare January 20, 2022 20:07
@domoritz domoritz changed the base branch from master to next February 25, 2022 13:50
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.
@arvind arvind marked this pull request as ready for review April 7, 2022 19:01
@arvind
Copy link
Member Author

arvind commented Apr 7, 2022

Woo hoo, with Vega 5.22 released, this PR is now ready for review which should unblock the much more exciting geo-interval (#6953) and lasso (#7960) features!

@arvind arvind requested a review from a team April 7, 2022 19:02
Copy link
Member

@domoritz domoritz left a 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)?

@arvind
Copy link
Member Author

arvind commented Apr 7, 2022

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.

@domoritz
Copy link
Member

domoritz commented Apr 7, 2022

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 1 or true as in https://github.com/vega/vega/blob/main/packages/vega-util/src/toSet.js. But yes, let's consider it for when we move selections to Vega.

@arvind arvind merged commit f2c38e6 into next Apr 7, 2022
@arvind arvind deleted the as/optimizedSelectionIDs branch April 7, 2022 22:07
@github-actions
Copy link

github-actions bot commented Apr 7, 2022

🚀 PR was released in v5.3.0-next.2 🚀

@github-actions github-actions bot mentioned this pull request Jun 21, 2022
BradyJ27 pushed a commit to BradyJ27/vega-lite that referenced this pull request Oct 19, 2023
Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants