Repository navigation
perf: resolve FromDI without a worker thread - #85
Merged
Merged
Conversation
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.
Every
FromDIwent through a worker thread on every message.Dependency.__call__tookcontext: faststream.ContextRepoto read the request container, and fast_depends resolves that parameter as a FastStreamContextcustom field. For each one it opens an anyio task group and runs the field's syncuse()throughanyio.to_thread.run_sync(fast_depends/core/model.py, thecustom_fieldsblock ofasolve).The request container now lives in a module-level
ContextVar. The middleware sets it aroundcall_nextand resets it with the token, andDependency.__call__takes no parameters. FastStream's own context locals are ContextVars as well, so each message, and each task it spawns, still sees its own container. The variable also cannot collide with a user's context names, which is the problem #74 fixed by renaming the key.The same change makes
setup_di's two startup hooksasync. FastStream ran them, as sync callables, in worker threads at every start, soinstall_middlewarewas changing broker config from a thread.Measured through
TestNatsBroker, median of 7 runs x 3000 messages, main and branch back to back:DependsFromDIFromDIThe machine was not quiet (load average about 3.6), so single cells are noisy. The 2-4x gap on the
FromDIrows is well outside that noise, and threeFromDInow cost about the same as three plainDepends.Two tests wrap
anyio.to_thread.run_syncin a mock and assert it is never called. One covers handling a message withFromDI, and it fails on main withassert 1 == 0. The other covers app startup and shutdown, and it fails on main withassert 2 == 0.Compatibility: the request container is no longer stored in the FastStream context under
modern_di_request_container. That key was a private constant, and nothing in the org reads it. The modern-di FastStream docs page mentions it in the 4.0.1 rename note, so after this is released that note will need a line saying which version dropped the key.