Skip to content

Shrink bigchunk data structure #1649

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

Merged
merged 5 commits into from
Sep 26, 2019
Merged

Shrink bigchunk data structure #1649

merged 5 commits into from
Sep 26, 2019

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented Sep 4, 2019

Remove pointer indirection on the underlying XORChunk, which will save memory and a few CPU cycles.
We have to be careful to create the Appender after the chunk is appended to our slice, since it has a pointer to the chunk memory.

And stop storing the end time of every small chunk - we can look at the start time of the next one since they don't overlap. This makes the data structure slightly smaller, and speeds up unmarshalling since we don't need to seek through every value.

We don't need to store the start time either, since it can be fetched from the underlying chunk, but that takes longer.

Estimate of the impact: Suppose chunks are avg 6 hours long then each one has 12 smallchunks and we save 16 bytes per = 192MB per million series, plus Go heap overheads.

@gouthamve gouthamve requested a review from tomwilkie September 5, 2019 11:46
@bboreham bboreham force-pushed the bigchunk-smaller branch 2 times, most recently from b07cc22 to e45e510 Compare September 13, 2019 16:45
@gouthamve gouthamve self-assigned this Sep 18, 2019
This will save memory and a few CPU cycles.
We have to be careful to create the Appender after the chunk is
appended to our slice, since it has a pointer to the chunk memory.

Signed-off-by: Bryan Boreham <[email protected]>
We can look at the start time of the next one since they don't
overlap.
This makes the data structure slightly smaller, and speeds up
unmarshalling since we don't need to seek through every value.

Signed-off-by: Bryan Boreham <[email protected]>
and add a test that covers that case.

Signed-off-by: Bryan Boreham <[email protected]>
Check it works after slice and unmarshal, and check it fails when you
seek off the end of the chunk.

Signed-off-by: Bryan Boreham <[email protected]>

fix test
It is only used to shortcut the case where FindAtOrAfter() is called
with a target past the end of the chunk, and this never happens
because we have from/through times on each chunk at a higher level.

Signed-off-by: Bryan Boreham <[email protected]>
Copy link
Contributor

@csmarchbanks csmarchbanks left a comment

Choose a reason for hiding this comment

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

LGTM

@bboreham bboreham merged commit 53f2043 into master Sep 26, 2019
@bboreham bboreham deleted the bigchunk-smaller branch September 26, 2019 12:31
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.

3 participants