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

Reduce memory footprint in UpdateBy rolling time operatations #5215

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

lbooker42
Copy link
Contributor

A timestamp value context was being retained for every bucket after its use expired. Replaced with a try-with-resources and modified the functions using it to accept the context as an input parameter.

@lbooker42 lbooker42 self-assigned this Mar 4, 2024
finalizeWindowBucket(context);
return;
}
try (final ChunkSource.GetContext tsContext = ctx.timestampColumnSource.makeGetContext(ctx.workingChunkSize)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the important line. Try-with-resources here and pass the context to the calling functions instead of keeping as a member.

Copy link
Contributor Author

@lbooker42 lbooker42 Mar 4, 2024

Choose a reason for hiding this comment

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

Github is having a problem with matching the lines, even though they are unchanged. Sorry for the confusion. Assuming it is the indentation.

Copy link
Member

@rcaudy rcaudy left a comment

Choose a reason for hiding this comment

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

This is obviously better, let's merge if unit tests are happy.

@rcaudy
Copy link
Member

rcaudy commented Mar 4, 2024

A "level up" would be to add WindowPreparationContext or similar that could allow this context creation to be shared across buckets that are being prepared on the same thread.

@lbooker42 lbooker42 merged commit 66b4be0 into deephaven:main Mar 4, 2024
23 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 4, 2024
@lbooker42 lbooker42 deleted the lab-oom-fix branch June 26, 2024 19:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants