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

[Backport release-2.27] Improve readers by parallelizing I/O and compute operations (#5401) #5451

Merged
merged 40 commits into from
Feb 14, 2025

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Feb 12, 2025

Backport of #5401 to release-2.27


TYPE: IMPROVEMENT
DESC: Improve readers by parallelizing I/O and compute operations

Shelnutt2 and others added 30 commits February 12, 2025 20:23
This removes the read from waiting on all I/O operations and instead
moves the I/O task to be owned by the datablock itself. If the I/O
threadpool task is valid, we block on data access. This lets I/O and
compute be interleaved by only blocking on data when its ready to be
processed and allows for better background data loading.
This allows for copying the task/future an enabled multiple threads to
check the status of the task in a thread-safe manner.
…checking.

While the ThreadPool::SharedTask is designed to be used by multiple
threads, its designed for copying. The data structure itself is not
thread safe.

A recursive mutext is needed because some functions like load_chunk_data
call back into filtered_data() and would deadlock. This could be handled by
also release the locking in load_chunk_data(), but a recursive_mutex is
used for better safety against deadlocks.
This is needed because we need to access the data buffer from inside the
unfiltering task to unfilter into. We can't block on unfiltering being
done from inside the unfiltering task so we need different accessors
which let us bypass the check on if the unfiltering task is completed.
This is needed because zip_coordinates is called from the unfilter task
itself.
Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

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

Procedural approve, haven't actually looked at the changes.

@ihnorton ihnorton merged commit 8652f02 into release-2.27 Feb 14, 2025
57 checks passed
@ihnorton ihnorton deleted the backport/pr-5401-to-release-2.27 branch February 14, 2025 13:24
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.

4 participants