Skip to content

Assemble loader tensors directly into their spill destination - #793

Merged
mkolodner-sc merged 1 commit into
mainfrom
mkolodner-sc/assemble-entity-tensors-into-spill-destination
Oct 9, 2026
Merged

mkolodner-sc merged 1 commit into
mainfrom
mkolodner-sc/assemble-entity-tensors-into-spill-destination

Conversation

@mkolodner-sc

Copy link
Copy Markdown
Collaborator

What

TFRecordDataLoader.load_as_torch_tensors assembles each entity's tensor by accumulating per-batch tensors in a list and joining them with tf.concat. The final tf.concat allocates the whole result while the list of batches is still alive, so the assembled tensor momentarily coexists with a second full-size copy of itself in anonymous memory.

When tensor spilling is enabled, this change allocates the full-size destination up front via allocate_disk_backed(...) and streams each batch into its slice, freeing the batch as it goes — so the result never coexists with a second full-size copy. When a disk-backed buffer is not used (spilling disabled, below the size threshold, or an unmapped dtype), it falls back to the existing tf.concat path unchanged, so behaviour is identical for callers that do not spill.

This is the "buffer that does not exist yet" case allocate_disk_backed already documents ("allocating the destination as the file from the start means those bytes never occupy anonymous memory at all") — the loader just wasn't using it for the final assembly yet.

Why

The assembly step is a peak-memory hotspot at load time. Measured on a 7.6 GiB fp32 shard (process RSS high-water-mark across the assembly step):

path peak RSS
tf.concat (before) 16.3 GiB
assemble into destination (this PR) 8.7 GiB

The transient second copy is removed — peak drops from ~2× the shard to ~1×.

Tests

Adds test_load_as_torch_tensors_assembles_into_spilled_destination: with spilling enabled the loaded ids / features / labels are byte-identical to the in-memory (tf.concat) result and are disk-backed; with spilling disabled, behaviour is unchanged. The existing dataloaders_test.py cases continue to cover the non-spilling (fallback) path.

@kmontemayor2-sc kmontemayor2-sc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks Matt! LGTM provided the comments are addressed :)

Comment thread gigl/common/data/dataloaders.py Outdated
Comment thread gigl/common/data/dataloaders.py Outdated
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/assemble-entity-tensors-into-spill-destination branch from a4e3f18 to 80d9d0c Compare October 9, 2026 18:21
@mkolodner-sc
mkolodner-sc added this pull request to the merge queue Oct 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 9, 2026
load_as_torch_tensors joined per-batch tensors with tf.concat, which
allocates the full result alongside the still-live list of batches -- a
transient second full-size copy in anonymous memory.

When tensor spilling is enabled, allocate the destination up front via
allocate_disk_backed and stream each batch into its slice, freeing the
batch as it goes, so the assembled tensor never coexists with a second
full-size copy. Fall back to tf.concat unchanged when a disk-backed
buffer is not used (spilling disabled, below the size threshold, or an
unmapped dtype).

Measured on a 7.6 GiB fp32 shard, peak process RSS for the assembly step
drops from 16.3 GiB to 8.7 GiB.
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/assemble-entity-tensors-into-spill-destination branch from 80d9d0c to 8bf29cd Compare October 9, 2026 20:14
@mkolodner-sc
mkolodner-sc added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit 036396a Oct 9, 2026
7 checks passed
@mkolodner-sc
mkolodner-sc deleted the mkolodner-sc/assemble-entity-tensors-into-spill-destination branch October 9, 2026 22:51
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.

3 participants