fix(core): prevent _configure_hooks accumulation in get_usage_metadata_callback#35330
Conversation
25d8c29 to
8525ce4
Compare
Merging this PR will degrade performance by 24.61%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_import_time[CallbackManager] |
286.4 ms | 323.5 ms | -11.48% |
| ❌ | WallTime | test_import_time[PydanticOutputParser] |
477.6 ms | 540.2 ms | -11.59% |
| ❌ | WallTime | test_import_time[tool] |
473.5 ms | 536.9 ms | -11.82% |
| ❌ | WallTime | test_import_time[BaseChatModel] |
482.7 ms | 540.7 ms | -10.73% |
| ❌ | WallTime | test_import_time[ChatPromptTemplate] |
549.8 ms | 635.1 ms | -13.44% |
| ❌ | WallTime | test_async_callbacks_in_sync |
18.4 ms | 24.4 ms | -24.61% |
| ❌ | WallTime | test_import_time[Runnable] |
444.5 ms | 495.8 ms | -10.35% |
Comparing gitbalaji:fix/core-configure-hooks-accumulation (e4ed9ba) with master (2d1492a)2
Footnotes
-
23 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
master(0b975d4) during the generation of this report, so 2d1492a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
CodSpeed benchmark regression is a false positiveThe bot flagged a 10.44% regression in Our change is in Verified locally: from langchain_core.prompts import ChatPromptTemplate
import sys
print('langchain_core.callbacks.usage' in sys.modules) # FalseThe module is not in The CodSpeed bot itself notes: "Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data." The 76ms variance (650ms → 726ms) is well within the noise range for subprocess-based import timing on shared GitHub Actions runners. |
cb233ac to
2dae84c
Compare
e4ed9ba to
17c743f
Compare
…a_callback Fixes langchain-ai#32300. Each call to `get_usage_metadata_callback()` previously created a new `ContextVar` and called `register_configure_hook`, appending to the global `_configure_hooks` list on every invocation. In long-running applications, this caused unbounded memory growth and increasingly slow callback configuration on every LLM call. Fix: lift the `ContextVar` and `register_configure_hook` call to module level so the hook is registered exactly once at import time. The context manager now sets/resets the single module-level var using `ContextVar.reset(token)`, which also correctly restores the previous handler in nested `get_usage_metadata_callback()` calls. The `name` parameter is deprecated (it only named the now-global `ContextVar`) and emits a `DeprecationWarning` when a non-default value is passed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ContextVar is invariant, so ContextVar[UsageMetadataCallbackHandler | None] cannot be passed to list.count() expecting ContextVar[BaseCallbackHandler | None]. Replace with an identity-based sum() to avoid the type mismatch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
17c743f to
b6577a1
Compare
Summary
Fixes #32300.
get_usage_metadata_callback()previously created a newContextVarand calledregister_configure_hookon every invocation, causing_configure_hooksto grow unboundedlyContextVarandregister_configure_hookcall to module level — registered exactly once at import timeContextVar.reset(token)instead ofset(None), which correctly restores the previous handler in nestedget_usage_metadata_callback()callsnameparameter is deprecated (it only named the now-globalContextVar) with aDeprecationWarningwhen a non-default value is passedAreas requiring careful review
_usage_metadata_callback_varsymbol is now public-ish (module-level with a_prefix); tests import it directly — maintainers may prefer a different approach to expose it for testingnameparameter deprecation: currently only warns when the value differs from the default ("usage_metadata_callback"); we could also warn on any usage of the parameterTest plan
test_configure_hooks_no_accumulation— asserts_configure_hooksdoes not grow after repeated callstest_name_parameter_deprecated— assertsDeprecationWarningwhen customnameis passedtest_name_parameter_default_no_warning— asserts no warning on default usagetest_nested_context_managers— asserts inner CM exit restores outer handler (notNone)test_usage_callbackandtest_usage_callback_asyncstill pass🤖 Generated with Claude Code