-
Notifications
You must be signed in to change notification settings - Fork 512
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
Reducing the memory footprint of active series custom trackers #2568
Merged
Merged
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
40bb6e7
Changing active series matches from []bool to map[int]bool for better…
gubjanos 3067b74
Changing map[int]bool to int slice
gubjanos 757f228
Changing slice allocation based on Oleg's suggestion
gubjanos 72ad043
Benchmark
gubjanos 10c353c
PoC: fixed size backed array slice for matchers
colega b152483
Adding Oleg's improvements as well
gubjanos 7cf417b
Limiting the maximum number of trackers as 64k
gubjanos ccccfd5
Making linter happy
gubjanos d0cb574
Merge branch 'main' into jg/active_series_memory_perf
gubjanos 779d0a8
Making preAllocDynamicSlice and also activeseries.mathches unexported
gubjanos f460b60
Commenting and cleaning up preAllocDynamicSlice
gubjanos da93b2f
Failing the benchmark if the config fails to initizalize
gubjanos 5693061
Updating changelog
gubjanos 26a4d6c
Update CHANGELOG.md
gubjanos 9def4f3
Update pkg/ingester/activeseries/matchers_test.go
gubjanos e2074d3
Merge branch 'main' into jg/active_series_memory_perf
gubjanos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can we see benchmarks of this? :)
This will have to allocate a new slice every
2^n
matchers, which is probably more expensive than just allocating a slice of bools since it will push unnecessary pressure on the GC.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.
If we're worried about memory consumption here, then IMO we should keep the previous approach and change
[]bool
to a bitarray backed by[]uint64