Repository navigation
Conversation
Mark every assert in the bench test modules with "# nosec B101", matching the convention of the other test files: pytest never runs under python -O, so the asserts are not stripped in practice. Also annotate the subprocess usage in test_jagged_sweep.py with "# nosec B404"/"# nosec B603". The test invokes sys.executable with a fixed argument list and no shell, so no untrusted input reaches the call; the suppression comments record that audit.
… nbit tests The int nbit lookup fixtures required exactly torch 2.14.0 and fbgemm-gpu-cpu 1.9.0, so every test using them errored on torch 2.14.1. Check only the major and minor versions, which matches the torch~=2.14.0 pin in pyproject.toml, and report the installed version when the check fails.
_copy_bytes_per_s copied from torch.empty, and freshly allocated device memory is typically zero-filled. Zeros are the best case for memory compression, so the copy can report more than the device's real bandwidth: on the BMG runner the streaming copy measured 518 GB/s, above the part's nominal ~456 GB/s. Fill the source with torch.rand so both copies in test_default_flush_evicts_last_level_cache measure incompressible traffic.
The fbgemm-xpu test step has hung on a runner with no output beyond the test file name, so there is no way to tell which call blocked. Pass -o faulthandler_timeout=600 to pytest: a test still running after 10 minutes dumps every thread's stack into the job log, which points at the blocking line. Reporting only; the hung test is not interrupted and the job still runs to its own timeout.
aagalleg
force-pushed
the
bench/jagged_ops
branch
from
October 7, 2026 17:41
e809373 to
c4fe413
Compare
dvrogozh
reviewed
Oct 7, 2026
| @@ -0,0 +1,594 @@ | |||
| # command: python -m fbgemm_xpu.bench.jagged_sweep --output jagged_tensor.csv | |||
Contributor
There was a problem hiding this comment.
I suggest to drop comments in the .csv file and have only actual data here. This way Github will actually be able to render the .csv file for us in a nice column way. It will actually become reviewable. If you need to save comments, then create a README.md in the folder and describe what each .csv file contains there.
…s limits The fbgemm-xpu test job runs two matrix entries at once on one BMG runner with a 12 GB GPU. Three things then went wrong, all in CI only. The flush test compared a copy sized to fit the last-level cache against a streaming copy 32x larger and expected them within 25%. That ratio encodes PVC behaviour: on GDDR6 a long read+write stream runs well below peak, while a short copy's writes are absorbed by the write-back cache, which no flush ahead of the copy can prevent. On BMG the two differ by 1.8x with the flush working correctly. Compare the same copy with and without the default flush instead; the cached copy must be at least 1.5x faster. PVC gives 4.3x, and upstream's fixed 40 MB flush still fails at 0.78x. The bench tests left 2.4 GiB in the caching allocator for the rest of the session, and the large-grid tests allocate 8 GiB operands behind a free-memory check that cannot see the other process. The driver backs allocations past physical memory with system memory instead of failing, so both jobs crawled rather than raised. Release the bench operands after each test, halve the bandwidth-bound test's working set to 1 GiB, and skip any large-operand test whose footprint exceeds half of the device's physical memory, leaving room for a second process. benchmark_torch_function sized its GPU lead from the slowest warm-up, which includes one-time kernel compilation (8-112 ms against a 20-80 us steady submission), so the lead hit its 50 ms cap, and upstream call sites queue it 1000 times: 50 s of GPU spin per measurement, up to 200 s with retries, and a GPU the other job could not use. Size the lead from the fastest warm-up, which the retry loop already covers if too small, and cap the lead queued across all iterations at 1 s. The patched upstream benchmark's smoke run drops from 130 s to 9 s with timings unchanged. Only warn about host waits once they reach half of the iterations, where they can move the median; a few jittery iterations in 1000 no longer flag every measurement.
c4fe413 passed -o faulthandler_timeout=600 to pytest so that a test still running after 10 minutes would dump every thread's stack and show where the job had stalled. The stall was two test processes oversubscribing the shared GPU's memory, and 00436ef removes its causes, so the job no longer needs the diagnostic. This reverts commit c4fe413.
test_default_flush_evicts_last_level_cache compares a half-LLC copy timed with and without the default flush and expects the flushed copy to be at least 1.5x slower. On the BMG runner the two came out equal at ~71 GB/s, 128 us for a 9 MB copy the part streams in ~25 us. The job shares its GPU with the other matrix entry, and time-slicing adds the same latency to both copies, so the cache effect the test looks for is buried whether or not the flush works. Reproduced on PVC with a matmul in another process: both copies at 43.6 ms, ratio 1.00. Add test_default_flush_is_twice_last_level_cache, which spies on Tensor.zero_ to check that the helper sizes its scratch buffer to twice the device's last-level cache and overwrites it before every timed iteration and not before warm-ups. This holds on any part and on a shared GPU, and still catches a return to upstream's fixed 40 MB. Have the bandwidth test measure a launch floor first, a one-element copy, and skip with the measured numbers when the cached copy is not clearly above it: either the GPU is shared or the cache is too small to resolve, and in both cases the comparison says nothing about the flush. On an idle PVC it still runs and passes with a 4.2x ratio.
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.
Adds a standalone
jagged-sweepbenchmark that verifies correctness against CPU before timing, patches the upstream FBGEMM jagged benchmark to run on XPU, commits PVC and BMG baselines with notes on interpreting them, and wires smoke runs into CI. Together these provide a reproducible performance reference for detecting regressions in the SYCL jagged kernels.Operator(s)
Benchmarked (forward and backward; previously implemented):
fbgemm::jagged_to_padded_dense— copies jagged values into a padded dense tensorfbgemm::jagged_2d_to_dense— 2D variant of jagged→dense conversionfbgemm::dense_to_jagged— gathers valid rows of a dense tensor into jagged formfbgemm::jagged_dense_elementwise_add_jagged_output— adds dense to jagged values, jagged outputChanges
jagged-sweepbenchmark insrc/fbgemm_xpu/bench/jagged_sweep.py: sweeps B ∈ {32…2048} × max_len ∈ {16…1024} × {float32, bfloat16, float16}, D = 128 (DLRM v3hstu_transducer_embedding_dim); every shape is verified against the CPU implementation before timing; emits CSV with a metadata headersrc/fbgemm_xpu/bench/bench_utils.py: XPU-event timing with last-level-cache flush and a GPU lead (torch.xpu._sleepspin) so the timed window never measures host submissionsrc/fbgemm_xpu/bench/skips.py0002-Add-XPU-support-to-fbgemm-jagged-benchmark.patchenabling the upstreamjagged_tensor_benchmark.py devicecase on XPUbench/baselines/pvc-max1550/jagged_tensor.csvandbench/baselines/bmg-b60/jagged_tensor.csv, plusbench/baselines/README.mddocumenting methodology, byte-count formulas, and known caveats (effective vs. physical bandwidth, latency floors, BMG memory compression)jagged-sweep(correctness gate) and the patched upstream benchmark on every run; validates the patch touches onlyfbgemm_gpu/bench/CONTRIBUTING.mdbenchmarking section andREADME.mdupdatestests/test_jagged_sweep.py,tests/test_bench_utils.py,tests/test_bench_skips.pyTesting
jagged-sweepcompares every forward output and input gradient against the CPU implementation withtorch.testing.assert_closebefore timing; the run fails and writes no CSV on any mismatch. All 576 sweep cases passed on both devices.Baseline run-to-run variation: PVC median 0.7% (max 5.5%), BMG median 0.1% (max 4.9%).
Hardware Validated
ZE_AFFINITY_MASK=0)OMP_NUM_THREADS=4