Skip to content

perf(cudf): Defer cache H2D past submission lock - #113

Merged
thirtiseven merged 2 commits into
HighPerfDataAccelerator:devfrom
thirtiseven:agent/defer-cudf-cache-h2d-submission
Sep 3, 2026
Merged

perf(cudf): Defer cache H2D past submission lock#113
thirtiseven merged 2 commits into
HighPerfDataAccelerator:devfrom
thirtiseven:agent/defer-cudf-cache-h2d-submission

Conversation

@thirtiseven

Copy link
Copy Markdown
Collaborator

Summary

  • Return a deferred future for cache-backed H2D work when GLUTEN_CUDF_CACHE_H2D_INLINE=1.
  • Keep the copy on the waiting caller thread while moving it out of cuDF's process-wide device-read submission critical section.
  • Add focused coverage using cuDF's public byte-range fetch helper and two independent data sources/streams.

Motivation

cuDF #22586 serializes device_read_async() submission through a process-wide mutex. The existing inline cache path performs cache lookup, cache-page traversal, pin retention, and H2D submission before returning a ready future, so all of that work runs while cuDF holds the mutex. Unrelated scan drivers then form a lock convoy.

This change returns a std::launch::deferred future instead. Creating the future is cheap; cuDF releases its submission mutex before waiting on it, and the actual copy still runs inline on the waiting caller thread. This preserves the intended inline execution model and cache-pin lifetime while restoring concurrency across callers.

This is a consumer-side mitigation for cuDF issue #23799. cuDF PR #23823 remains the cleaner owner-side fix; this shim should be re-evaluated once a qualified runtime contains that change.

Validation

  • Test-enabled cuDF target build: 1000/1000 targets completed.
  • Focused tests passed: directCachePageH2dAvoidsHostStaging and deferredCacheH2dDoesNotHoldCudfSubmissionMutex (2/2).
  • AWS EMR on EKS, 4 x g7.8xlarge, Parquet SF1000 on S3, strict cold Q1-Q22, one iteration per arm.
  • Same Spark-Gluten, cuDF, configuration, data, and retained cluster; the Velox patch was the only source difference.
  • Baseline: 132.006 s, 22/22 execution and same-run semantic-digest checks passed.
  • Patched: 123.840 s, 22/22 execution and same-run semantic-digest checks passed.
  • Delta: -8.167 s (-6.19%, 1.066x); 17/22 queries improved.

The recovered 8.167 seconds closely matches the independently isolated 8.068-second GLUTEN_CUDF_CACHE_H2D_INLINE=1 regression introduced by cuDF #22586. This change addresses that mutex convoy only; it does not claim to eliminate every performance difference between cuDF releases.

@github-actions github-actions Bot added the cudf label Sep 2, 2026
@thirtiseven thirtiseven changed the title perf(cudf): defer cache H2D past submission lock perf(cudf): Defer cache H2D past submission lock Sep 2, 2026
@sperlingxx
sperlingxx self-requested a review September 2, 2026 14:22

@sperlingxx sperlingxx 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.

LGTM. Great catch!

The deferred policy will not launch a NEW thread for asynchronous execution. Instead, it put off the execution to escape the scope of cuDF's global mutex device_read_mutex elegantly.

Velox gives BufferedInputDataSource to cuDF
              ↓
cuDF fetch_byte_ranges_to_device_async_impl()
              ↓
cuDF acquires device_read_mutex()
              ↓
cuDF calls datasource.device_read_async()
              ↓ virtual dispatch
Velox BufferedInputDataSource::device_read_async()

@thirtiseven
thirtiseven merged commit cf619d9 into HighPerfDataAccelerator:dev Sep 3, 2026
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants