Repository navigation
[ML] Avoid copies and redundant event loops in the RDataLoader - #23761
Merged
siliataider merged 8 commits intoOct 20, 2026
Merged
Conversation
siliataider
force-pushed
the
rdataloader-opt-batching
branch
from
October 19, 2026 15:45
af5c1c2 to
1a51c27
Compare
siliataider
changed the title
siliataider
marked this pull request as ready for review
October 19, 2026 15:52
siliataider
requested review from
guitargeek,
hageboeck and
vepadulano
as code owners
October 19, 2026 15:52
vepadulano
requested changes
Oct 19, 2026
vepadulano
left a comment
vepadulano
left a comment
Member
There was a problem hiding this comment.
Thanks! Some comments from my side.
Test Results 23 files 23 suites 3d 23h 24m 38s ⏱️ For more details on these failures, see this check. Results for commit f137c16. ♻️ This comment has been updated with latest results. |
The batches read the rows of a chunk or dataset in shuffled order, instead of copying it into a shuffled tensor first. The leftover rows are appended to the leftover batch directly, which is moved to the queue once full. Assisted-by: Claude Opus 5.5
…sets The training and validation datasets, and the sampled ones, are filled by reading the loaded rows in shuffled order, instead of building shuffled, sliced and concatenated copies first. The datasets of several dataframes are split directly into the concatenated ones. Assisted-by: Claude Opus 5.5
The filtered eager path ran one event loop per passing entry, after a `Take` of `rdfentry_` to find them. A single event loop over the filtered dataframe loads the same entries, and the buffer is reserved from the cluster metadata like in the unfiltered case. Assisted-by: Claude Opus 5.5
The first epoch read each filtered training cluster twice: once to collect `rdfentry_` and find the training/validation boundary, then again to load the training rows. The entries are now taken in the same event loop that loads the cluster, and the validation rows are dropped from the buffer. Assisted-by: Claude Opus 5.5
`SampleWithReplacement` created and seeded a new generator for every sample, which dominated the oversampling epochs and, with a fixed seed, drew the same sample every time. Assisted-by: Claude Opus 5.5
The eager epochs copied the whole dataset into batches before returning the first one, so the dataset and all its batches were in memory together. Each batch is now gathered from the dataset when it is requested. Assisted-by: Claude Opus 5.5
siliataider
force-pushed
the
rdataloader-opt-batching
branch
from
October 19, 2026 19:56
1a51c27 to
5956f72
Compare
vepadulano
approved these changes
Oct 20, 2026
vepadulano
left a comment
vepadulano
left a comment
Member
There was a problem hiding this comment.
Thank you! LGTM, see minor suggestion.
The dataframes are loaded into a single dataset, and the training and validation sets, as well as the classes of the samplers, are lists of its rows. The split no longer copies the dataset, nor do the samplers, and the batches gather the rows of their set. Assisted-by: Claude Opus 5.5
… every entry `RActionCustomLoader::CallExec` allocated a new vector of value addresses for every entry. Assisted-by: Claude Opus 5.5
siliataider
force-pushed
the
rdataloader-opt-batching
branch
from
October 20, 2026 09:22
5956f72 to
f137c16
Compare
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
2 participants
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.
This Pull request:
Follow-up of #23642, with one commit per optimization of the
RDataLoadermemory usage and runtime, in the eager and lazy paths, with and without filters.Benchmarking results
7256529 rows x 11 cols, batch 262144, 10 epochs, baseline RSS 0.26 GiB, run on a local machine