Repository navigation
fix dataloader worker busy-polling and add worker cleanup - #42
Open
curtischong wants to merge 4 commits into
Open
curtischong wants to merge 4 commits into
curtischong wants to merge 4 commits into
Conversation
Workers looped on index_queue.get(timeout=0) / continue, burning ~17% of a core each while idle (~23k wakeups/s). The parent had the same spin on output_queue. Blocking get() drops idle CPU to zero and cuts round-trip latency 5-16x. Also add DataLoader.shutdown() so the None sentinel is actually sent and workers are joined instead of living until process exit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
curtischong
commented
Jul 29, 2026
| output_queue.put((index, dataset[index])) | ||
| # allow exit without flushing queued results, otherwise a mid-iteration | ||
| # shutdown deadlocks the join on unflushed results | ||
| output_queue.cancel_join_thread() |
Author
There was a problem hiding this comment.
from the docs https://docs.python.org/3/library/multiprocessing.html#multiprocessing.Queue.cancel_join_thread
A better name for this method might be allow_exit_without_flush(). It is likely to cause enqueued data to be lost, and you almost certainly will not need to use it. It is really only there if you need the current process to exit immediately without waiting to flush enqueued data to the underlying pipe, and you don’t care about lost data.
curtischong
marked this pull request as ready for review
July 29, 2026 15:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While training nequix I noticed that index_queue.get(timeout=0) makes the workers continuously poll (because of the timeout=0). By removing this timeout, the workers block until they receive a message in the queue.
This is a benchmark run by claude:
Sidenote: we might also want to update https://teddykoker.com/2020/12/dataloader to reflect this change.
This PR also adds explicit shutdown calls to the workers
This PR was tested to train on 1500 matprj structures on 110 training steps