Repository navigation
Metal: spread long graphs over more command buffers (GPU watchdog) - #44
danielhanchen wants to merge 4 commits into
Conversation
(cherry picked from commit 04fa42e44e3a0998a4dced5937a2316c8aea6c95)
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d58d6518fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -53,6 +53,7 @@ struct ggml_metal { | ||
|
|
||
| // command buffer state | ||
| int n_cb; // number of extra threads used to submit the command buffers |
There was a problem hiding this comment.
This patch's context does not match the pinned ggml commit 135ce1e0e3f2b0693d3de93d4f6b5fa0a1947820: for example, that revision has no blank line between capture_scope and // command buffer state, or between the fusion-statistics block and GGML_LOG_INFO. The earlier patches do not modify this file, so they cannot resolve these mismatches. The source-preparation step in .github/workflows/unsloth-sd-prebuilt.yml:231–233 applies every patch unconditionally, which means this addition stops prebuilt release preparation for every platform, not just Metal. Regenerate the hunks against the pinned source and verify the complete patch sequence with git apply --check (documented by local git apply -h as “see if the patch is applicable”).
Useful? React with 👍 / 👎.
Problem
On Metal, a long graph is encoded into too few command buffers. A Z-Image 512 px denoise step then runs long enough to trip the macOS GPU watchdog: on a GitHub macos-15 runner (M1 virtual GPU) 0 of 18 renders finished.
Fix
ggml-patches/0021spreads a big graph over more command buffers. Numbered 0021 because 0014-0020 are taken by open PRs (0018 is the CPU iqk GEMM patch in #48, 0019 the Metal im2col patch in #45, 0020 the SVE patch in #36); patches apply in glob order, and 0019 + 0021 apply together on ggml135ce1e0. Master merged in on Nov 7.Evidence
GitHub macos-15 (Apple M1 virtual, 3 cores, 7 GB), Z-Image 512 px on Metal: master 0 / 18 renders complete, this branch 4 / 4. Speed is a tie on the graphs that already ran, and the images are identical.
Not covered
No real Apple Silicon (P/E cores, more memory); one virtual GPU only.