fix(pipeline): eliminate double execution and worker idle race - #74
Closed
simonCatBot wants to merge 1 commit into
Closed
fix(pipeline): eliminate double execution and worker idle race#74simonCatBot wants to merge 1 commit into
simonCatBot wants to merge 1 commit into
Conversation
Two related bugs introduced by the streaming extension were causing the pipeline conformance tests (especially ManualSchedule/4 loop_count=1000) to hang or timeout in CI: 1. vxExecuteGraph ran every graph twice. The pipeup warm-up loop set steady_done after the first iteration and only broke on the next loop check, so non-pipeup graphs executed twice. Add ownGraphHasPipeup() and only request the extra steady iteration when the graph actually contains a node with pipeup_output_depth > 1. 2. vxWaitGraph could return while the worker was mid-batch. The pipeline worker decremented in_flight to 0 between back-to-back frames, creating a window where vxWaitGraph saw 'idle' even though the worker was about to start the next frame. Add worker_is_processing and only clear it / signal idle when the worker truly has no more work. Local tests: - GraphPipeline fast: 37/37 pass - GraphPipeline.ManualSchedule/4: passes - GraphStreaming: 24/24 pass - Graph.*: 233/233 pass
Contributor
Author
|
Opened against the wrong repo — the fix belongs to ROCm/MIVisionX#1722. |
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
1 participant
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.
Summary
Fixes two bugs in the streaming/pipelining interaction that were causing the
GraphPipelineconformance tests (notablyManualSchedule/4withloop_count=1000) to hang or time out in CI.Bug 1:
vxExecuteGraphran every graph twiceThe pipeup warm-up loop in
vxExecuteGraphwas unconditional:steady_done = true.!any_pipeup && steady_doneand broke.For normal graphs without pipeup-depth nodes this executed the graph twice. In the pipeline worker that meant each queued frame ran twice, roughly doubling the stress-suite run time.
Fix: add
ownGraphHasPipeup()and only request the extra "one steady iteration" when the graph actually contains a node withpipeup_output_depth > 1.Bug 2:
vxWaitGraphcould return while the worker was still mid-batchThe pipeline worker runs queued frames back-to-back in one inner loop. It increments
in_flightat batch start and decrements it after each frame. There was a window wherein_flightwas momentarily0between frames even though the worker was about to start the next one.vxWaitGraphonly checkedin_flight > 0, so it could return during that window. The test then recycled buffers while the worker was still using them, desynchronising the queues and eventually hanging.Fix: add a
worker_is_processingflag to the graph and makevxWaitGraphwait underpipe_lockuntil!worker_is_processing && in_flight == 0. The worker clears the flag only when it is truly about to go idle.Verification
GraphPipeline.*fast tests: 37/37 passGraphPipeline.ManualSchedule/4(loop_count=1000): passesGraphStreaming.*: 24/24 passGraph.*(non-pipeline graph tests): 233/233 passI also ran
ManualSchedule/450 times in a loop locally without a hang.Notes
The latest Copilot review on PR #69 was addressed in commit
0b944d6; this PR is a follow-up on top of that merged state.