Skip to content

Speed up the coroutine-function check in watcher creation - #178

Closed
Korijn wants to merge 1 commit into
masterfrom
claude/observ-gc-hook-refactor-8lbab4
Closed

Speed up the coroutine-function check in watcher creation#178
Korijn wants to merge 1 commit into
masterfrom
claude/observ-gc-hook-refactor-8lbab4

Conversation

@Korijn

@Korijn Korijn commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #177, recovering part of the watcher-creation overhead that was accepted there.

What

Watcher.__init__ calls inspect.iscoroutinefunction twice — once for the watched function and once for the callback. Each call goes through the generic inspect machinery (functools.partial unwrapping, method and function-like checks), which profiling showed accounts for ~500 ns per watcher creation. For plain functions and methods — by far the common case — the answer is available directly from the callable's code flags.

This adds a fast-path iscoroutinefunction helper in observ.watcher that checks __code__.co_flags & CO_COROUTINE directly and only falls back to inspect.iscoroutinefunction for callables without a __code__ attribute (e.g. functools.partial, custom callables) or ones explicitly marked with inspect.markcoroutinefunction (Python ≥ 3.12). The helper is also used in weak(), so bound-method watchers benefit as well.

Correctness

A new parity test asserts the helper agrees with inspect.iscoroutinefunction for every kind of callable that can be watched: plain sync/async functions, lambdas, bound sync/async methods, callable instances, partials wrapping sync/async functions, builtins, and (on 3.12+) markcoroutinefunction-decorated functions.

Result

Watcher creation goes from 5957 → 5116 ns/op (~14% faster) on the test_watcher_creation benchmark setup, measured back-to-back against master on the same machine.

Full test suite passes (210 tests incl. the Qt group), ruff check/format clean.

Note: this narrows, but does not fully close, the test_watcher_creation gap accepted in #177 — the Benchmarks gate may still flag that one benchmark against the pre-#177 baseline's spirit, though this PR's own baseline is post-#177 master, against which it is a strict improvement.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T4cFbKSCxjGabtJbB9e5H2


Generated by Claude Code

Watcher.__init__ calls inspect.iscoroutinefunction twice (once for
the watched function, once for the callback), and each call walks the
generic inspect machinery: unwrapping functools.partials, method
checks and function-like checks. That is wasted work for plain
functions and methods, which are by far the common case.

Add a fast-path helper that answers directly from the callable's code
flags and only falls back to inspect for callables without a __code__
attribute (e.g. functools.partial) or ones explicitly marked with
inspect.markcoroutinefunction. A parity test checks the helper against
inspect.iscoroutinefunction for every kind of callable that can be
watched.

This makes watcher creation ~14% faster (5957 -> 5116 ns/op on the
watcher_creation benchmark setup), recovering part of the overhead
that was accepted with #177.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4cFbKSCxjGabtJbB9e5H2

Korijn commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Benchmarks failure diagnosis: gate noise, not a regression

The failed Benchmarks job is not caused by this PR. Evidence:

  1. The flagged benchmarks don't exercise the changed code. CI flagged test_proxy_creation_dict, test_write_list_append, test_write_set_add, test_write_dict_new_key — none of which create watchers — and, tellingly, test_read_list_iterate[plain] / test_read_dict_iterate_values[plain], which iterate raw unproxied containers and run zero observ code.
  2. test_watcher_creation — the only benchmark this PR touches — passed (it's the one that improves, ~14% locally).
  3. The flagged set changes on every run. Reproducing the workflow locally flagged a different subset (write_list_append[100k] instead of [10], no [plain] reads), with deltas that are a fraction of one standard deviation (e.g. proxy_creation_dict[10]: mean 11.5 → 12.4 µs, StdDev 2.7 µs, 30 rounds).
  4. Master vs. master fails the gate. Running the exact compare procedure with identical code on both sides still reports "regressions" up to +18% (proxy_creation_dict[1k]). The gate's noise floor is well above the 5% threshold for these µs-scale benchmarks at 30 rounds.

So the --benchmark-compare-fail=mean:5% gate is currently below the noise floor of the suite/runners and will red-flag most PRs regardless of content (it also failed on #177 for benchmarks unrelated to that change, e.g. test_read_list_iterate[reactive] at 5.47%).

Suggestions (happy to PR any of these)

  • Mark the Benchmarks job advisory with continue-on-error: true so the comparison table stays available without blocking, and/or
  • Raise the threshold to something above the observed noise (e.g. mean:25%) so only genuine regressions like the one caught on Manage proxy_db lifetimes with reference counting instead of a gc hook #177 (+57%) trip it, and/or
  • Increase rounds / use --benchmark-min-time for the sub-microsecond benchmarks to tighten variance.

No changes planned to this PR itself — the code is unaffected by the failure.


Generated by Claude Code

@Korijn Korijn closed this Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants