Repository navigation
[ML] De-template the RDataLoader engine to cut the JIT cost - #23642
Merged
Merged
Conversation
siliataider
marked this pull request as draft
October 7, 2026 10:27
siliataider
force-pushed
the
rdataloader-opt
branch
from
October 7, 2026 10:29
87a2e76 to
21e9fba
Compare
Test Results 24 files 24 suites 4d 2h 43m 24s ⏱️ For more details on these failures, see this check. Results for commit 05a7e89. ♻️ This comment has been updated with latest results. |
siliataider
force-pushed
the
rdataloader-opt
branch
7 times, most recently
from
October 9, 2026 15:08
480740d to
f9f88f6
Compare
siliataider
marked this pull request as ready for review
October 9, 2026 15:24
vepadulano
requested changes
Oct 14, 2026
vepadulano
left a comment
Member
There was a problem hiding this comment.
Thank you! Here are some comments from my side.
siliataider
force-pushed
the
rdataloader-opt
branch
from
October 16, 2026 13:43
19c5a47 to
be7f7cf
Compare
Add a RAction-derived class, together with an accompanying action helper, that serve the purpose of loading a variable number of columns into a target array of floating-point values. The action and the helper do not require compile-time knowledge of the column types. These classes are meant for internal use only, specifically to be used by the RDataLoader.
Vector columns are truncated or padded to their maximum size. Assisted-by: Claude Opus 5.5
Assisted-by: Claude Fable 5
The padding was truncated to an unsigned integer in eager mode, so negative or fractional values were wrong. Assisted-by: Claude Opus 5.5
siliataider
force-pushed
the
rdataloader-opt
branch
from
October 16, 2026 14:40
be7f7cf to
2c27ef4
Compare
The loaders read the columns with the custom RDF loader action. Assisted-by: Claude Opus 5.5
Assisted-by: Claude Fable 5
Assisted-by: Claude Fable 5
Only `RDataLoaderEngine.hxx` and `RFlat2DMatrix.hxx`, which Python uses, stay public, so fewer headers are parsed when the engine is looked up from Python. Assisted-by: Claude Opus 5.5
Assisted-by: Claude Opus 5.5
`LoadCustomValues` appends to a buffer owned by the caller instead of returning a new one, so the data loader no longer copies the values into its staging buffers and datasets, nor zero-fills them beforehand. Assisted-by: Claude Opus 5.5
siliataider
force-pushed
the
rdataloader-opt
branch
from
October 16, 2026 15:55
2c27ef4 to
05a7e89
Compare
vepadulano
approved these changes
Oct 16, 2026
vepadulano
left a comment
Member
There was a problem hiding this comment.
Thank you, great work! I don't have further comments on the changes. Let's see it validated with a couple of CI runs.
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.
This Pull request:
Changes or fixes:
De-template the RDataLoader by adding a custom RDataFrame action to load the entries as the RDataLoader needs them.
Benchmarking results
7256529 rows x 11 cols, batch 262144, 10 epochs, baseline RSS 0.26 GiB, run on a local machine
Flow
%%{init: {"flowchart": {"nodeSpacing": 20, "rankSpacing": 30}, "themeVariables": {"fontSize": "13px"}}}%% flowchart LR PY["RDataLoader(...)"] --> ENG PY -.->|"before (master)"| OLD["template string<br/>→ cling jits per schema"] subgraph ML["[ML] compiled engine + loaders"] ENG["engine ctor:<br/>column layout once"] --> LOAD["cluster / dataset loader<br/>(cols, vecSizes, vecPadding)"] end subgraph RDF["[RDF] type-erased custom loader action"] CALL["LoadCustomValues"] --> HELP["once per column:<br/>T → AppendAs⟨T⟩<br/>RVec⟨T⟩ → AppendVectorAs⟨T⟩"] HELP --> EXEC["each entry: void* per column<br/>→ handler → floats<br/>(vectors truncated / padded)"] end LOAD --> CALL EXEC --> COPY["copy into RFlat2DMatrix<br/>→ batches"] classDef old stroke-dasharray:5 5 class OLD old