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

Skip conversion of contexts into vectors #89

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Conversation

crisptrutski
Copy link
Collaborator

It turns out there's a big opportunity to expose the scopes directly as Java objects rather than vectors.

I tried instead just interning the conversions, but that turned out quite complex and to still use quite a bit more memory. Since we don't actually use the contexts for anything externally, this seems fine. The API has added some methods to read their properties without interop.

This is a non-breaking change for Metabase.

Before all these context optimizations:

> (user/time+ (complex-benchmark))
Time per call: 77.74 ms   Alloc per call: 111,128,445b   Iterations: 26

Afterwards (incl previous PR)

> (user/time+ (complex-benchmark))
Time per call: 43.94 ms   Alloc per call: 29,220,062b   Iterations: 47

@crisptrutski
Copy link
Collaborator Author

A big part of the win is probably not building new sequences. I tried caching the cons -> sequence mappings as well and also didn't help much.

@crisptrutski crisptrutski merged commit 4fa033b into master Aug 22, 2024
4 checks passed
@crisptrutski crisptrutski deleted the context-native branch August 22, 2024 10:45
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.

2 participants