Skip to content

♻️ Create dependency-cache dict in solve_dependencies only if None (don't re-create if empty)#13689

Merged
tiangolo merged 2 commits into
fastapi:masterfrom
bokshitsky:remove-deps-cache-copy
Sep 20, 2025
Merged

♻️ Create dependency-cache dict in solve_dependencies only if None (don't re-create if empty)#13689
tiangolo merged 2 commits into
fastapi:masterfrom
bokshitsky:remove-deps-cache-copy

Conversation

@bokshitsky

@bokshitsky bokshitsky commented May 6, 2025

Copy link
Copy Markdown
Contributor

Hello,
Current code:

  1. Creates new dependency_cache dictionary inside solve_dependencies function if passed dependency_cache parameter is None or empty.
  2. As a result of (1) dependency_cache.update(solved_result.dependency_cache) is required at every iteration of dependencies solving, because solved_result.dependency_cache returned by recursion can be either same as dependency_cache (if at least one dependency was already stored in cache) or new dict (on first iterations of recursion where empty cache is passed to recursive function)

This behaviour adds a bit complexity to the code and spends (albeit a little) more CPU and memory resources.

My proposal:

  1. always create dependency cache on first iteration of recursive dependency solving and pass created cache instance to all subsequent calls. Do not exchange cache dict on first steps of recursion iterations if passed dict was empty.
  2. Now dependency_cache.update(solved_result.dependency_cache) is not required, because one dependency_cache is used over all dependency solving process.

@bokshitsky bokshitsky force-pushed the remove-deps-cache-copy branch from f64d254 to a30f83c Compare May 6, 2025 21:53
@bokshitsky bokshitsky force-pushed the remove-deps-cache-copy branch from a30f83c to 7c0a1e6 Compare May 6, 2025 21:58
@bokshitsky bokshitsky changed the title remove non-required dependency-cache dict copy and update 🔧 remove non-required dependency-cache dict copy and update May 6, 2025
@bokshitsky bokshitsky changed the title 🔧 remove non-required dependency-cache dict copy and update 🔧 remove non-required dependency-cache dicts creation and update May 7, 2025
@bokshitsky

This comment was marked as resolved.

@bokshitsky

Copy link
Copy Markdown
Contributor Author

@YuriiMotov sorry for mention you, can you please give any advice that can I do to move this PR further?

I think these changes a pretty safe and improve a bit performance and mem-print, although in my company we need them to remove hacks relates to manual solve_dependencies usage out of fastapi-http-scope

@YuriiMotov YuriiMotov changed the title 🔧 remove non-required dependency-cache dicts creation and update ♻️ Create dependency-cache dict in solve_dependencies only if None (don't re-create if empty) Jun 28, 2025

@YuriiMotov YuriiMotov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

These changes make code clearer and less error-prone.

@bokshitsky, I would recommend you update the description to make it more concise

@bokshitsky

Copy link
Copy Markdown
Contributor Author

@YuriiMotov thanks, I updated, I think changes are clearer now.

@bokshitsky

Copy link
Copy Markdown
Contributor Author

@tiangolo hello, may you look please - can I do something to include patch in following releases?

@tiangolo tiangolo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! 🚀

Thanks for the detailed explanation. 🍰

@tiangolo tiangolo merged commit c2c6049 into fastapi:master Sep 20, 2025
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants