serializer: dict-based handler dispatch + transport hot-path log cleanup (+ tests) - #4
serializer: dict-based handler dispatch + transport hot-path log cleanup (+ tests)#4Salman778 wants to merge 5 commits into
Conversation
`serialize` ran on every outbound frame and reflected on the frame's
class to locate its handler:
handler = getattr(self, f"_frame_{type(frame).__name__.lower()}", None)
if callable(handler):
result = handler(frame)
if inspect.isawaitable(result):
return await result
return result
Each call paid for an f-string allocation, a `str.lower()`, a `getattr`,
a `callable` check, and an `inspect.isawaitable` walk - per frame. For
outbound audio this fires dozens of times per second per call.
Switch to two pre-built dispatch tables keyed by frame class, populated
in `__init__`:
self._sync_frame_handlers: dict[type, ...] # EndFrame, CancelFrame, ...
self._async_frame_handlers: dict[type, ...] # OutputAudioRawFrame
`serialize` becomes a single `type(frame)` lookup against each table.
Because the sync/async split is known at registration time we no longer
need `inspect.isawaitable` on the hot path - sync handlers return their
result directly, async ones are awaited.
The old reflective form matched only the exact class name (it
used `type(frame).__name__`, not `isinstance`), so an exact-type lookup
is behavior-preserving. Unhandled frames still produce a `logger.trace`
and return None.
Also switches the unhandled-frame trace log to loguru's positional-arg
form so the format string isn't expanded when TRACE is filtered out.
`_handle_event` previously located its handler via reflection:
handler = getattr(self, f"_ev_{message_type.lower()}", None)
if callable(handler):
return handler(message)
Same pattern as the frame dispatch refactor: f-string allocation +
`str.lower()` + `getattr` + `callable` check, per event.
Events from chan_websocket are rare compared to audio frames (one
MEDIA_START per call, occasional DTMF, etc.), so this is mostly a
consistency change rather than a measured hot-path win. But it's the
same shape as the frame-handler refactor, so the dispatch table fits
naturally alongside the sync/async frame tables and the explicit
registration makes the supported event set easier to see at a glance.
A new `_event_handlers: dict[str, ...]` is populated in `__init__` and
keyed by the exact event-name string Asterisk sends. The lookup is a
single dict get. Unknown events fall through to the same `logger.info`
"unhandled event" branch as before. Both the warning and info log calls
now use loguru's positional-arg form so the format string isn't
expanded when the level is filtered out.
Also drops the unused `Any` and `cast` imports left over from the
previous reflective form.
…o_frame Two small cleanups on the outbound write path: - All `logger.warning` / `logger.error` / `logger.trace` / `logger.debug` / `logger.info` calls in `_media_start_handler` and `write_audio_frame` used eager f-strings. Python builds the formatted string before the call even reaches loguru, so the work happens on every call even when the level is filtered out at the sink. Switch to loguru's positional-arg form so a filtered call drops the formatting work entirely. This matches the same change made on the hot-path log lines in flow_controller in PR NikolayShakin#3. - Replace `type(payload) == bytes` with `isinstance(payload, bytes)` in the post-serialize guard. `isinstance` is the conventional Python check, slightly faster in CPython 3.12+, and correctly accepts `bytes` subclasses if any ever appear in this code path. No behavioral changes - just smaller per-frame cost when logging is filtered and a more idiomatic type check.
Adds the first tests for AsteriskFrameSerializer plus `pytest` and `pytest-asyncio` in the dev dependency group (this is the first test file in the repo, so the pytest config block is also new). 15 cases covering the public surface of the new dispatch: Frame dispatch: - Every sync handler (EndFrame, CancelFrame, InterruptionFrame, AsteriskCommandFrame) is reachable through `serialize()` and produces the expected wire-format command. - The async handler (OutputAudioRawFrame) is awaited and returns the audio bytes through the no-resampling fast path. - Sync handlers return a plain string, never a coroutine - a regression guard for the sync/async split that lets us drop the per-frame `inspect.isawaitable` check. - Unhandled frame types (e.g. StartFrame) fall through to None. Event dispatch: - DTMF_END produces InputDTMFFrame. - QUEUE_DRAINED produces InputTransportMessageFrame carrying the message. - MEDIA_XOFF / MEDIA_XON dispatch to their handlers (which log and return None - we check the dispatch found them, not the log output). - Messages without an `event` field return None. - Unknown event types return None. Dispatch-table integrity: - All event-handler keys are uppercase (matching chan_websocket's wire format). - Sync and async frame-handler tables have no overlapping keys (would be ambiguous dispatch). Run with `uv run pytest`.
|
@Salman778, that's another great contribution, thanks! |
|
@NikolayShakin This is not a great contribution. There is zero possibility that someone has such wide and unfocused interests in low-level internals, combined with such immaculate use of Markdown formatting conventions and such inorganic formulations of natural language. The author does not understand his PR. This is an AI slop PR, and unfortunately, every open-source project is inundated with them right now. They should be treated like spam. |
| # the values produced by the protocol parser. All handlers are sync | ||
| # so one table is enough; same goal as the frame tables - skip the | ||
| # per-event `getattr` + `f"_ev_{name.lower()}"` lookup. | ||
| self._event_handlers: dict[str, Callable[[dict], Frame | None]] = { |
There was a problem hiding this comment.
The idea behind _event_something methods is to make it more convenient for users.
If at some point we need to add a handler for some new event, it's enough to just add _ev_my_custom_message method.
But I agree, it's inefficient. Actually, in the first iteration I used a dict as handler registry.
However, I would create this dictionary in a loop iterating through all the methods of the class, in init method.
like this:
class test():
def __init__(self):
self.handlers = dict()
for k, v in inspect.getmembers(self.__class__, predicate=inspect.isfunction):
if k.startswith("_ev_"):
self.handlers[k] = v
def _ev_0(self): ...
def _ev_a(self): ...
def _ev_b(self): ...
def _ev_c(self): ...
def _something_a(self): ...
This would solve the perf issue and doesn't affect the mechanism of adding _ev_ prefixed handlers
| # `inspect.isawaitable` check on the hot path. Async handlers are | ||
| # kept in a separate table so we know at lookup time whether to | ||
| # `await` the result. | ||
| self._sync_frame_handlers: dict[type, Callable[[Frame], str | bytes | None]] = { |
There was a problem hiding this comment.
The same idea as here
We can parse the handlers once in init to avoid overhead on every frame, save them into sync/async dicts. And splitting the handlers into sync and async is a good point; I didn't think about it
Keeps the dict dispatch, isinstance check, and static-string cleanups; only the variable-bearing log calls go back to f-strings, matching the maintainer's feedback on NikolayShakin#3
fe80ba2 to
a495530
Compare
|
Pushed: reverted the log calls back to f-strings per the convention from #3. Everything else unchanged, all tests passing. |
|
Implemented in #8 |
serializer: dict-based handler dispatch + transport hot-path log cleanup (+ tests)
Summary
Follow-up to #3, focused on the serializer side of the audio path.
Three independent commits + tests:
perf: dict-based frame handler dispatch inserialize().serializeruns on every outbound frame and previously reflected onthe frame's class:
Each call paid for an f-string allocation, a
str.lower(), agetattr, acallablecheck, and aninspect.isawaitablewalk —per frame. For outbound audio that's dozens of times per second per
call.
Replaced with two pre-built dispatch tables (sync + async) keyed by
frame class, populated in
__init__.serializebecomes a singletype(frame)lookup against each table. Because the sync/asyncsplit is known at registration time,
inspect.isawaitableis gonefrom the hot path entirely.
The old reflective form matched only the exact class name
(
type(frame).__name__, notisinstance), so an exact-type lookupis behavior-preserving.
perf: dict-based event handler dispatch in_handle_event().Same shape, applied to the event side. Events are rare compared to
audio frames, so this is mostly a consistency change rather than a
measured hot-path win — but the explicit registration makes the
supported event set obvious at a glance, and removes the matching
getattr(... f"_ev_{name.lower()}")reflection.perf: lazy log formatting +isinstancecheck intransport.write_audio_frame/_media_start_handler. Mirrorsthe loguru lazy-formatting change made on the flow_controller hot
path in Flow controller perf and close fix #3, applied to the per-frame error/warning logs in the
transport. Also replaces
type(payload) == byteswithisinstance(payload, bytes)for the conventional check.Tests
Adds
tests/test_serializer.py(15 cases) pluspytestandpytest-asyncioin thedevdependency group. Coverage:Frame dispatch:
EndFrame,CancelFrame,InterruptionFrame,AsteriskCommandFrame) is reachable viaserialize()and producesthe expected wire-format command.
OutputAudioRawFrame) is awaited and returnsaudio bytes through the no-resampling fast path.
guard for the sync/async split.
StartFrame) fall through toNone.Event dispatch:
DTMF_ENDproducesInputDTMFFrame;QUEUE_DRAINEDproducesInputTransportMessageFrame.MEDIA_XOFF/MEDIA_XONdispatch to their handlers (which logand return
None— we check the dispatch found them).None.Dispatch-table integrity:
wire format).
Run with
uv run pytest.Compatibility / breaking changes
None. The public API of
AsteriskFrameSerializerandAsteriskWebsocketTransportis unchanged. The dispatch refactor ispurely internal; the old reflective form was exact-match by class
name, and the new tables are exact-match by class — same behavior.
If #3 lands first, this PR rebases cleanly on top of it (the two PRs
touch different files; the only shared change is the
pytest/pytest-asynciodev-deps and[tool.pytest.ini_options]block in
pyproject.toml, which trivially merges).Background
Same context as #3 — auditing the audio path of a production voice
agent. After the flow_controller changes in #3,
serializewas thenext thing showing up as measurable per-frame work on the outbound
side; the
getattr/isawaitablereflection was small per call butfires hundreds of times per second across a fleet of pods.