[tornadovm] Replace the dummy forward pass in forceCopyInReadOnlyData() with TornadoExecutionPlan.transferToDevice() - #148
Open
mikepapadim wants to merge 1 commit into
Open
Conversation
…dummy forward pass to upload weights
orionpapadakis
added a commit
that referenced
this pull request
Aug 28, 2026
…dings The handoff still told the next session to retarget #129, assess #142 and expect three conflicting PRs. All four landed on 2026-08-28, so that guidance would have sent someone to do work that is done — the same failure mode the 2026-08-03 notes had, which this file already warns about. - The pending-PR table becomes a landed table: what each cost to adapt, and which PRs are genuinely still open (#131, #146, #148, and #149 which is this branch). - The merge guidance is gone rather than kept as history. It described a decision — which base to merge into — that has been made and acted on. - Records that CI had been dead for a month, why a green code-quality check hid it, and that build-and-run.yml draws from a pool with both a Mac and a Linux runner, so an unpinned job can pass on one and fail on the other. - Records the CUDA Q8_0 batch-prefill skip and the TornadoVM issue behind it. - Git state: the branch is pushed and open as draft PR #149, its history was rewritten, and force-pushing now needs asking because it is no longer local-only. Immediate next actions now say plainly that M6 cannot start until D-10, D-12 and D-14 close, and lists what is actually actionable meanwhile: the T1.7 recording call, wiring the benchmark gate into CI (only #146 still in the way), and taking the packed-half2 numerics question back to #138. Adds the defects found while landing the four PRs, with what they share: each was invisible because the thing that would have caught it was not running — no golden for that family, no CI, or no test exercising that overload. Architecture link validation: 326 internal links across the docs all resolve.
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.
What this changes
forceCopyInReadOnlyData()exists to get the model weights onto the GPU. It did that by running a full forward pass on zeroed state — the activation graph, then every layer graph, then the logits graph — purely for the copy-in side effect, because there was no way to ask for a transfer without running something:TornadoVM now has the operation this method was faking, so the whole thing becomes:
Applied to all three plan variants —
TornadoVMMasterPlanSingleToken,TornadoVMMasterPlanPrefillDecode,TornadoVMMasterPlanBatchPrefillDecode.With CUDA graphs enabled the dummy pass is also the capture, so it is kept as-is in that case. The change is a guard clause; the
--cuda-graphspath is untouched, and the benchmarks below confirm that.Benefits
1. The copy-in phase gets faster. It no longer runs
N+2graphs' worth of kernels it does not want. Llama-3.2-3B-Instruct-Q8_0, RTX 4090, JDK 21, CUDA backend, 3 runs each:main(dummy forward pass)transferToDevice())~10.6% off the copy-in phase, and it is the more honest kind of saving: the phase is dominated by the upload itself (host-register + PCIe), so removing the kernels removes essentially all of what was not the transfer.
2. Inference performance is unchanged — which is the point. The change only moves when data arrives, not how the model runs.
llama-bench-style,-p 128 -n 128 -r 5 --no-warmup:main--with-prefill-decodemain--with-prefill-decode--cuda-graphs(path untouched)main--cuda-graphs(path untouched)Token generation is a tie in every configuration, and the
--cuda-graphsnumbers are identical to three significant figures, as they should be for a code path this PR does not touch.3. The method now says what it means. A method whose name is "copy in read-only data" is a copy-in, not a forward pass on zeroed state. It also stops depending on the forward pass being safe to run on garbage input — a constraint nobody was tracking, which quietly rules out anything that would divide by a zeroed norm or index from a zeroed position.
One honest caveat
For a short single-shot generation the reported
tok/sdrops, because the dummy pass used to install and first-launch every kernel, and the API path leaves that for the first real token. It is the same work moving, not new work:mainBy a few hundred tokens it is parity, and end-to-end wall clock is the same or marginally better at every length measured (the startup saving offsets the first-launch cost). Nothing gets slower overall — the cost just shows up on a different counter. Fixing that properly means a warm-up that installs code without transferring, which is worth doing separately and is easier to build now that the transfer exists as its own operation.
Testing
mvn test— 16/16 pass.default,--with-prefill-decode,--cuda-graphs) with Llama-3.2-3B-Instruct-Q8_0.--bench -p 128 -n 128 -r 5 --no-warmup, run againstmainand this branch built identically, same machine, same TornadoVM build.--batch-prefill-size 128fails identically onmainand on this branch in the dev build used here (a TornadoVM codegen error unrelated to this change), so that path is unverified either way rather than regressed.Environment: RTX 4090, Ubuntu 24.04, JDK 21, TornadoVM CUDA backend.