[PR #113] [MERGED] Fix malloc graph search skipping overlapping alias ranges #122

New issue

Closed

opened 2026-10-26 22:24:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:24:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-aimdo/pull/113
Author: @localbobthebuilder
Created: 9/13/2026
Status: ✅ Merged
Merged: 9/15/2026
Merged by: @rattus128

Base: master ← Head: fix/alias-range-search

📝 Commits (3)

293de00 Fix malloc graph search skipping overlapping alias ranges
9ea5e09 Assert physical page reuse in overlapping alias regression
cc93975 Reuse cleanup scan to find the earlier conflicting alias

📊 Changes

2 files changed (+42 additions, -3 deletions)

View changed files

📝 src/malloc-graph.c (+6 -3)
➕ tests/malloc-graph-overlapping-alias-cuda.py (+36 -0)

📄 Description

Fix allocation search skipping valid overlapping alias ranges

The malloc-graph allocator can extend its virtual range even when an existing
range satisfies the allocation. This wastes virtual address space and can
contribute to exhausting the fixed arena while physical graph memory remains
well below that limit.

During a candidate scan, physical_live includes both pre-existing live pages
and pages temporarily marked live by that scan. When a repeated physical page
is encountered within the candidate, advancing by j + 1 skips overlapping
windows that no longer contain the earlier occurrence.

For example, with the free mapping [0, 1, 2, 0, 1, 3] and a four-page request,
the window starting at virtual index 2 is valid: [2, 0, 1, 3]. The current
search misses it. Advance past the earlier occurrence on a temporary alias
collision; retain the existing advancement otherwise. This preserves the
requirement that a returned range contain distinct, available physical pages.

The standalone GPU regression creates that layout with small allocations:
stock grows virtual space from 48 to 80 MiB, while the correction reuses 48 MiB.
Physical space is 32 MiB in both cases. Distinct values per page check that
the returned range has no unintended physical aliasing. No model is required.

Validation on Windows / RTX 5090, Torch 2.14.0+cu130, AIMDO 0.5.3:

A CPU harness using the actual stock/candidate search loops and a brute-force
reference checked 2,866,560 mapping/live/rogue-mask cases. Stock missed 2,364
ranges; the candidate missed zero.
Seven existing native tests passed on stock and candidate: fragmentation,
branch reuse, reordered free, variable subgraph replay, rogue alias, rogue
event splice and changed size.
The existing aliasing test's peak assertion fails on both stock and candidate
(expected 56 MiB, observed 48 MiB). This is disclosed as a baseline mismatch;
assertions after that failure have not been counted as passing.

The exact submitted GPU regression was verified failing on stock and passing on
the patched native build. Existing GPU tests were run with
PYTORCH_ALLOC_CONF=backend:cudaMallocAsync so their allocation hooks are active.

Contribution agreement

I agree that these contributions are licensed under GPLv3.
I grant Comfy Org the relicensing rights described in CONTRIBUTING.md.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-aimdo/pull/113 **Author:** [@localbobthebuilder](https://github.com/localbobthebuilder) **Created:** 9/13/2026 **Status:** ✅ Merged **Merged:** 9/15/2026 **Merged by:** [@rattus128](https://github.com/rattus128) **Base:** `master` ← **Head:** `fix/alias-range-search` --- ### 📝 Commits (3) - [`293de00`](https://github.com/Comfy-Org/comfy-aimdo/commit/293de007b8d19c354f7dac3bc7f2e70abf597fe8) Fix malloc graph search skipping overlapping alias ranges - [`9ea5e09`](https://github.com/Comfy-Org/comfy-aimdo/commit/9ea5e09110bdbe7ca0b59872e528f49e7d63c254) Assert physical page reuse in overlapping alias regression - [`cc93975`](https://github.com/Comfy-Org/comfy-aimdo/commit/cc939758fada84282edd7aa5626d61e35c68c040) Reuse cleanup scan to find the earlier conflicting alias ### 📊 Changes **2 files changed** (+42 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/malloc-graph.c` (+6 -3) ➕ `tests/malloc-graph-overlapping-alias-cuda.py` (+36 -0) </details> ### 📄 Description # Fix allocation search skipping valid overlapping alias ranges The malloc-graph allocator can extend its virtual range even when an existing range satisfies the allocation. This wastes virtual address space and can contribute to exhausting the fixed arena while physical graph memory remains well below that limit. During a candidate scan, `physical_live` includes both pre-existing live pages and pages temporarily marked live by that scan. When a repeated physical page is encountered within the candidate, advancing by `j + 1` skips overlapping windows that no longer contain the earlier occurrence. For example, with the free mapping `[0, 1, 2, 0, 1, 3]` and a four-page request, the window starting at virtual index 2 is valid: `[2, 0, 1, 3]`. The current search misses it. Advance past the earlier occurrence on a temporary alias collision; retain the existing advancement otherwise. This preserves the requirement that a returned range contain distinct, available physical pages. The standalone GPU regression creates that layout with small allocations: stock grows virtual space from 48 to 80 MiB, while the correction reuses 48 MiB. Physical space is 32 MiB in both cases. Distinct values per page check that the returned range has no unintended physical aliasing. No model is required. Validation on Windows / RTX 5090, Torch 2.14.0+cu130, AIMDO 0.5.3: - A CPU harness using the actual stock/candidate search loops and a brute-force reference checked 2,866,560 mapping/live/rogue-mask cases. Stock missed 2,364 ranges; the candidate missed zero. - Seven existing native tests passed on stock and candidate: fragmentation, branch reuse, reordered free, variable subgraph replay, rogue alias, rogue event splice and changed size. - The existing aliasing test's peak assertion fails on both stock and candidate (expected 56 MiB, observed 48 MiB). This is disclosed as a baseline mismatch; assertions after that failure have not been counted as passing. The exact submitted GPU regression was verified failing on stock and passing on the patched native build. Existing GPU tests were run with `PYTORCH_ALLOC_CONF=backend:cudaMallocAsync` so their allocation hooks are active. ## Contribution agreement - [x] I agree that these contributions are licensed under GPLv3. - [x] I grant Comfy Org the relicensing rights described in CONTRIBUTING.md. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:24:46 +00:00

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfy-aimdo#122

No description provided.

Rows
Columns