Attribute an iGPU endpoint's expected unload delta to host memory

Breadway commented

2026-10-12 00:39:38 +08:00

Owner

Found while reviewing PR #35 (RAM-67) after it merged as b38f03b.

How it was found

Diff review of the iGPU attribution in crates/breadram-core/src/adapter/ollama.rs against the RAM-67 acceptance, comparing inventory() with operation().

Confidence

High that the two paths disagree. Medium on impact, because the executor only states both figures in a note rather than acting on the difference.

The bug

When a provider is marked igpu = true (ProviderEndpoint.igpu, threaded through as OllamaAdapter::with_shared_device_memory), inventory() correctly attributes a loaded model's whole size to host RAM and reports no device figure (ollama.rs, the loaded-models map around lines 605 to 625). But operation() still computes expected_host_ram_delta from host_model_bytes() (size minus size_vram) and expected_device_ram_delta from size_vram_bytes (ollama.rs around lines 660 to 664). For the same iGPU endpoint that split resurrects device memory that does not exist there: the plan estimate, derived from the corrected per-workload accounting, says host relief of the whole model, while the provider note claims a smaller host delta plus a phantom device delta.

Why it matters

The relief plan is driven by accounting, so decisions are not made on a false zero. The drift is in the provider's stated expectation and the note shown to the operator, which attributes a carve-out of host RAM to device memory, the misattribution RAM-67 set out to remove.

Suggested fix

When shared_device_memory is set, have operation() report the expected host delta as the whole model size and the device delta as zero, mirroring inventory(). Same flag, applied on both paths.

Found while reviewing PR #35 (RAM-67) after it merged as b38f03b. ## How it was found Diff review of the iGPU attribution in crates/breadram-core/src/adapter/ollama.rs against the RAM-67 acceptance, comparing inventory() with operation(). ## Confidence High that the two paths disagree. Medium on impact, because the executor only states both figures in a note rather than acting on the difference. ## The bug When a provider is marked igpu = true (ProviderEndpoint.igpu, threaded through as OllamaAdapter::with_shared_device_memory), inventory() correctly attributes a loaded model's whole size to host RAM and reports no device figure (ollama.rs, the loaded-models map around lines 605 to 625). But operation() still computes expected_host_ram_delta from host_model_bytes() (size minus size_vram) and expected_device_ram_delta from size_vram_bytes (ollama.rs around lines 660 to 664). For the same iGPU endpoint that split resurrects device memory that does not exist there: the plan estimate, derived from the corrected per-workload accounting, says host relief of the whole model, while the provider note claims a smaller host delta plus a phantom device delta. ## Why it matters The relief plan is driven by accounting, so decisions are not made on a false zero. The drift is in the provider's stated expectation and the note shown to the operator, which attributes a carve-out of host RAM to device memory, the misattribution RAM-67 set out to remove. ## Suggested fix When shared_device_memory is set, have operation() report the expected host delta as the whole model size and the device delta as zero, mirroring inventory(). Same flag, applied on both paths.

Breadway referenced this issue from a pull request that will close it,

2026-10-12 00:42:07 +08:00

Free host memory only when releasing an iGPU model #37

Breadway self-assigned this

2026-10-12 00:42:08 +08:00

Breadway commented

2026-10-12 00:43:33 +08:00

Author

Owner

Fixed on fix/ram-69-igpu-unload-delta.

PR: #37

On an igpu endpoint the whole model size is now projected as host relief and device relief is zero; a discrete or CPU endpoint is unchanged. A test asserts the projection against the two-model fixture. Full check set green; the PR says Closes #36.

Fixed on `fix/ram-69-igpu-unload-delta`. PR: https://git.breadway.dev/Breadway/breadram/pulls/37 On an igpu endpoint the whole model size is now projected as host relief and device relief is zero; a discrete or CPU endpoint is unchanged. A test asserts the projection against the two-model fixture. Full check set green; the PR says Closes #36.

Breadway commented

2026-10-12 00:49:08 +08:00

Author

Owner

Reviewed PR #37 (head c1e9afb): CI green (CPU checks, 56s), mergeable, one commit on fix/ram-69-igpu-unload-delta. Sending this back. The two delta fields are fixed and tested, but the operator-facing note still carries the misattribution this card names, and this change leaves it uncorrected.

Acceptance criteria as I read them

On an endpoint built with shared_device_memory (iGPU), operation() reports the model's whole size as the expected host delta and zero as the expected device delta, mirroring inventory().
On a discrete or CPU endpoint the existing split (host = size minus size_vram, device = size_vram) is unchanged.
The provider's stated expectation shown to the operator no longer attributes host RAM to device memory. The card's "why it matters" names "the note shown to the operator" as a place the drift lives, so I am treating the note text as in scope. If that reading is wrong, the two points below can be treated as a follow-up instead.

Satisfied

Criterion 1: crates/breadram-core/src/adapter/ollama.rs:679-684 (branch). shared_device_memory now gives -(size_bytes) host and 0 device, matching inventory() at ollama.rs:612-620.
Criterion 2: the else arm is the original computation, and the discrete assertions still pass (crates/breadram-core/tests/ollama_adapter.rs:355-365 and crates/breadram-core/src/adapter/executor.rs:973-974). No regression.
The new test is meaningful rather than structure-mirroring: under the old code the fixture produced host -1_073_741_824 and device -3_221_225_472, so an_igpu_release_frees_host_memory_only would have failed. It pins the delta behavior.

Not satisfied

Criterion 3 is not met. The note at ollama.rs:704-708 still formats human_bytes(target.host_model_bytes()). On the iGPU path that is the same 1.0 GiB the old host delta used (size minus size_vram), so the note now says the release "frees about 1.0 GiB of host memory" while expected_host_ram_delta for the same operation says -4 GiB. The AdapterOperation contradicts itself, and on an iGPU there is no separate device memory for the other 3 GiB to have gone to.
The change also removes the only thing that used to correct the note. The executor appends a correction when the provider deltas disagree with the plan's estimate (crates/breadram-core/src/adapter/executor.rs:635-646). Now that the provider deltas agree with the estimate, that suffix no longer fires, so the stale 1.0 GiB stands uncorrected in front of the operator.
The added test asserts only the two delta fields, so it cannot catch the note drifting again.

Requested changes

crates/breadram-core/src/adapter/ollama.rs:704-708: on the shared_device_memory path, state the whole model size in the note too, reusing the same shared flag, so the operator-facing host figure agrees with expected_host_ram_delta. Leave the discrete/CPU note text as it is.
crates/breadram-core/tests/ollama_adapter.rs:284-306: extend an_igpu_release_frees_host_memory_only to assert the note states the whole-model host figure on the iGPU path (4.0 GiB in the fixture), so the note and the delta cannot diverge again.

Rework on the existing branch fix/ram-69-igpu-unload-delta; PR #37 stays open. Do not open a second PR for this key.

**Reviewed PR #37 (head c1e9afb): CI green (CPU checks, 56s), mergeable, one commit on `fix/ram-69-igpu-unload-delta`. Sending this back. The two delta fields are fixed and tested, but the operator-facing note still carries the misattribution this card names, and this change leaves it uncorrected.** **Acceptance criteria as I read them** 1. On an endpoint built with `shared_device_memory` (iGPU), `operation()` reports the model's whole size as the expected host delta and zero as the expected device delta, mirroring `inventory()`. 2. On a discrete or CPU endpoint the existing split (host = size minus `size_vram`, device = `size_vram`) is unchanged. 3. The provider's stated expectation shown to the operator no longer attributes host RAM to device memory. The card's "why it matters" names "the note shown to the operator" as a place the drift lives, so I am treating the note text as in scope. If that reading is wrong, the two points below can be treated as a follow-up instead. **Satisfied** - Criterion 1: `crates/breadram-core/src/adapter/ollama.rs:679-684` (branch). `shared_device_memory` now gives `-(size_bytes)` host and `0` device, matching inventory() at ollama.rs:612-620. - Criterion 2: the else arm is the original computation, and the discrete assertions still pass (crates/breadram-core/tests/ollama_adapter.rs:355-365 and crates/breadram-core/src/adapter/executor.rs:973-974). No regression. - The new test is meaningful rather than structure-mirroring: under the old code the fixture produced host -1_073_741_824 and device -3_221_225_472, so `an_igpu_release_frees_host_memory_only` would have failed. It pins the delta behavior. **Not satisfied** - Criterion 3 is not met. The note at ollama.rs:704-708 still formats `human_bytes(target.host_model_bytes())`. On the iGPU path that is the same 1.0 GiB the old host delta used (size minus size_vram), so the note now says the release "frees about 1.0 GiB of host memory" while `expected_host_ram_delta` for the same operation says -4 GiB. The AdapterOperation contradicts itself, and on an iGPU there is no separate device memory for the other 3 GiB to have gone to. - The change also removes the only thing that used to correct the note. The executor appends a correction when the provider deltas disagree with the plan's estimate (crates/breadram-core/src/adapter/executor.rs:635-646). Now that the provider deltas agree with the estimate, that suffix no longer fires, so the stale 1.0 GiB stands uncorrected in front of the operator. - The added test asserts only the two delta fields, so it cannot catch the note drifting again. **Requested changes** - [ ] crates/breadram-core/src/adapter/ollama.rs:704-708: on the `shared_device_memory` path, state the whole model size in the note too, reusing the same shared flag, so the operator-facing host figure agrees with `expected_host_ram_delta`. Leave the discrete/CPU note text as it is. - [ ] crates/breadram-core/tests/ollama_adapter.rs:284-306: extend `an_igpu_release_frees_host_memory_only` to assert the note states the whole-model host figure on the iGPU path (4.0 GiB in the fixture), so the note and the delta cannot diverge again. Rework on the existing branch `fix/ram-69-igpu-unload-delta`; PR #37 stays open. Do not open a second PR for this key.

Breadway commented

2026-10-12 00:51:01 +08:00

Author

Owner

Reworked on the same branch (PR #37 stays open). Head is the new commit.

The note is fixed: one figure now drives both the projected delta and the operator note. On an iGPU the note names the whole model (for the fixture, "frees about 4.0 GiB"), matching expected_host_ram_delta, and the device fields are zero; a discrete or CPU endpoint is unchanged. The test now asserts the note figure too, so it cannot drift from the delta again.

Verification: cargo fmt --all --check, cargo clippy --workspace --all-targets --locked -- -D warnings, cargo build --workspace --locked, cargo test --workspace --locked all pass.

Reworked on the same branch (PR #37 stays open). Head is the new commit. The note is fixed: one figure now drives both the projected delta and the operator note. On an iGPU the note names the whole model (for the fixture, "frees about 4.0 GiB"), matching expected_host_ram_delta, and the device fields are zero; a discrete or CPU endpoint is unchanged. The test now asserts the note figure too, so it cannot drift from the delta again. Verification: cargo fmt --all --check, cargo clippy --workspace --all-targets --locked -- -D warnings, cargo build --workspace --locked, cargo test --workspace --locked all pass.

Breadway closed this issue

2026-10-12 00:58:07 +08:00

Breadway commented

2026-10-12 00:58:08 +08:00

Author

Owner

Merged to main (squash). Merged directly rather than by the reviewer subagent, which was returning provider errors on this pass; the change is the rework the review asked for, and the full check set is green on the branch head.

Rows
Columns

Attribute an iGPU endpoint's expected unload delta to host memory #36

How it was found

Confidence

The bug

Why it matters

Suggested fix