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

Back MutableAntichain by ChangeBatch #505

Merged

Conversation

frankmcsherry
Copy link
Member

This PR removes the Vec<(T, i64)> from MutableAntichain and replaces it with a ChangeBatch<T>. There is a regression here in that several "unsafe" methods involving dirty bits are removed. Their justification was around progress tracking, and that justification no longer exists. This removes several ways in which one could write incorrect code (by dirtying things up and then reading data without first cleaning). These are now prevented.

Some potential for performance regressions in that certain loops are not exited as early as they are elsewhere, and the eager compaction work may not be helpful in all cases.

cc: @teskje

fixes #504

Copy link
Contributor

@benesch benesch left a comment

Choose a reason for hiding this comment

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

Well, makes sense to me, for whatever that's worth! Seems like a good thing to slip into Materialize at the start of a release cycle (e.g., now), so we can get some nightly coverage with the new implementation.

Copy link
Member

@antiguru antiguru left a comment

Choose a reason for hiding this comment

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

LGTM!

timely/src/progress/frontier.rs Show resolved Hide resolved
timely/src/progress/frontier.rs Outdated Show resolved Hide resolved
@frankmcsherry frankmcsherry merged commit 5b808ff into TimelyDataflow:master Feb 21, 2023
teskje added a commit to teskje/materialize that referenced this pull request Feb 21, 2023
This is to pick up
TimelyDataflow/timely-dataflow#505, which fixes
a memory leak in environmentd.
teskje added a commit to MaterializeInc/materialize that referenced this pull request Feb 21, 2023
This is to pick up
TimelyDataflow/timely-dataflow#505, which fixes
a memory leak in environmentd.
@github-actions github-actions bot mentioned this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make MutableAntichain consolidate even without frontier advancement
3 participants