Replace busy-wait publishers in ResampleConcat and ResampleUnit with event-driven publishing - #178
Merged
Merged
Conversation
The output() publisher polled next(processor) in a loop with asyncio.sleep(0), spinning a full core whenever no output was ready. Publish instead from the two subscriber handlers, draining the processor after each push. This is lossless because the composed resampler is always reference-driven (resample_rate=None), so output readiness only changes on new input -- the wall-clock max_chunk_delay extrapolation applies to prescribed-rate mode only. Also preserves backpressure: a subscriber does not complete until its outputs are published. Adds an integration test driving the unit through a live graph with interleaved reference/signal chunks, asserting output flows and the published time axis stays monotonic with both handlers publishing.
gen_resampled polled next(processor) with asyncio.sleep(0), spinning a core whenever no output was ready. Wait instead on an asyncio.Event set by both input handlers (cleared before draining so a push landing mid-drain re-arms the wait). Unlike ResampleConcat, this unit cannot publish from its handlers alone: in prescribed-rate mode with a finite max_chunk_delay, output is meant to become ready by wall clock with no input, so the event wait uses that delay as a timeout in that mode. Note: the wall-clock extrapolation itself is currently unreachable at the processor level (after a drain the source buffer retains ~2 samples and __next__ returns early on src.available() < 3 before evaluating b_project) -- true of the previous polling design as well. The timed wake preserves the intended trigger for when that guard is reworked; the new integration test documents this. Adds integration tests driving ResampleUnit through a live graph in both reference-driven and prescribed-rate modes.
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.
Summary
ResampleConcat.output()andResampleUnit.gen_resampled()both pollednext(self.processor)in awhile Trueloop withawait asyncio.sleep(0)when no output was ready.sleep(0)yields to the event loop but never sleeps, so each unit spun a full CPU core whenever idle. This PR makes both units publish only when output can actually exist.ResampleConcat: drain from the subscriber handlers
The polling publisher is removed entirely.
on_reference/on_signaleach push to the processor and then drain it, publishing every chunk it can currently produce (same idiom asAlign). This is lossless because the composed resampler is always reference-driven (resample_rate=None), and in that mode output readiness only ever changes on new input — the wall-clockmax_chunk_delayextrapolation applies to prescribed-rate mode only. A singlenext()consumes all currently-eligible reference values, so the drain loop runs at most twice per message.Side benefit: backpressure is preserved — a subscriber does not complete until its outputs are published, so a slow consumer backs up the input queues instead of silently growing the resampler's internal buffers.
ResampleUnit: event wake with a timeout in prescribed-rate mode
This unit cannot publish from its handlers alone: in prescribed-rate mode with a finite
max_chunk_delay, output is intended to become ready by wall clock with no input. The publisher therefore waits on anasyncio.Eventset by both input handlers, usingmax_chunk_delayas await_fortimeout whenresample_rateis set and the delay is finite. The event is cleared before draining so a push landing mid-drain re-arms the next wait rather than being lost.on_signalis overridden via@ez.subscriber(BaseConsumerUnit.INPUT_SIGNAL)(the same patternDownsampleuses) to add the wake.Pre-existing issue surfaced (not fixed here)
While testing the timed wake I found that the
max_chunk_delaywall-clock extrapolation is currently unreachable at the processor level, and was under the old polling design too: after a drain the source buffer retains only ~2 samples, andResampleProcessor.__next__returns early onsrc.available() < 3before evaluatingb_project. The timed wake preserves the intended trigger for when that guard is reworked (linearinterp1donly needs 2 points, so relaxing the guard for the extrapolation path looks feasible); the new integration test documents this and can be extended to assert extrapolated output once fixed.Tests
tests/integration/ezmsg/test_resampleconcat_system.py(new): drivesResampleConcatthrough a liveez.rungraph with interleaved reference/signal chunks at 100 vs 99.7 Hz; asserts output flows, has A+B channels, and that the published time axis stays strictly monotonic with both handlers publishing to the same stream. Terminates on quiet, which also proves nothing depends on a background publisher task.tests/integration/ezmsg/test_resample_system.py(new): drivesResampleUnitin reference-driven and prescribed-rate modes; asserts output flows and is monotonic, with a documented pointer for the extrapolation assertion above.