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.

Sign up for GitHub

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

Jump to bottom

Re-use memory for chunks in ingester QueryStream #3177

Merged
merged 5 commits into from Sep 15, 2020
Merged

Re-use memory for chunks in ingester QueryStream #3177

merged 5 commits into from Sep 15, 2020

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented Sep 14, 2020 •
edited

This improves performance of queries and reduces garbage-collection.

It works by making an array of reusable objects, so that they all stick around until the batch-send function is called.
If there are many batches then the savings can be considerable.

Benchmark (using 2000 series so about 15 batches):

before:
BenchmarkIngester_QueryStream-4              154           7448975 ns/op        16886320 B/op      32108 allocs/op
after:
BenchmarkIngester_QueryStream-4              187           6027117 ns/op         3771120 B/op      28647 allocs/op

Relates to #3171, but I don't think this will fix the issue.

Checklist

  • Tests updated
  • NA Documentation added
  • CHANGELOG.md updated

@bboreham
Refactor: extract function to set up ingester benchmark data
c9e93c8 
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
@bboreham
Add benchmark for ingester QueryStream()
98421d0 
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
@bboreham
Re-use memory for chunks in ingester QueryStream
04f676c 
This improves performance of queries and reduces garbage-collection.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
@bboreham
Add CHANGELOG entry
95adc02 
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
pracucci
pracucci approved these changes Sep 15, 2020
View reviewed changes
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

pstibrany
pstibrany approved these changes Sep 15, 2020
View reviewed changes
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@pracucci pracucci merged commit bb6784c into master Sep 15, 2020
@pracucci pracucci deleted the querystream-reuse-mem branch September 15, 2020 13:17
@bboreham bboreham mentioned this pull request Sep 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pstibrany pstibrany pstibrany approved these changes

@pracucci pracucci pracucci approved these changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bboreham @pstibrany @pracucci