⚡️ Skip doing any work when sub dependant is already cached by xkabylgSICKAG · Pull Request #11323 · fastapi/fastapi · GitHub

Hi, we noticed very large performance gains after making this change for our app that heavily uses the dependency injection system. One of our slower endpoints went from taking ~1100ms to ~400ms.

It looks really reasonable to check whether the sub_dependency is already solved and cached before solving it!

Related PR, that uses almost the same approach: #4089 (closed)

@YuriiMotov

Related PR, that uses almost the same approach: #4089

That PR also has a test! :)

I think that other PR, with your suggested changes, is a better change than this PR. Do you think it makes more sense to close this in favour of the other one?

I've rebased and updated the PR with the suggested changes from @YuriiMotov and with the change to dependency_cache empty dict checking from #4089.

@svlandeg Sure! I can spend some time on that later tonight. I would be very happy to get this merged so we can stop running a forked version of fastapi 🙂

@svlandeg I updated the code to add that test from #4089. I would have liked to create a similar test that didn't use include pydantic, but I wasn't able to figure out how to do that and still test the same thing.

I verified that the test fails before this and passes after this.

Are there any reasons not to simplify it this way?

Suggested change

      
                        # Use a unique object to compare against in case the cached value is None
          
                        cache_miss = object()
          
                        cached_value = dependency_cache.get(sub_dependant.cache_key, cache_miss)
          
                        # If the sub dependant is already cached, skip doing any more work
          
                        if cached_value is not cache_miss:
          
                            if sub_dependant.name is not None:
          
                                values[sub_dependant.name] = cached_value
          
                            continue
          
                        if sub_dependant.cache_key in dependency_cache:
          
                            cached_value = dependency_cache[sub_dependant.cache_key]
          
                            if sub_dependant.name is not None:
          
                                values[sub_dependant.name] = cached_value
          
                            continue

dict.get(...) is atomic operation, and thus thread safe, while consequent key in dict; dict[key] is not

Good point!
But currently dependency solving process is not parallel.
Also, it's highly unlikely (useless?) that somebody decide to delete existing key from dependency_cache.

I would vote for clarity in this case if nobody brings other arguments

We have been running this fork in our production application for quite a while now, and before we changed to an atomic .get() we got hard to reproduce KeyErrors.

LGTM

I still believe we can safely simplify this part, but it's not critical

I still believe we can safely simplify this part, but it's not critical

I would have thought it could be simplified too, but we did get KeyErrors before introducing that change to an atomic operation. It only happened a couple of times a week in total, but that's still too many times. 🤷‍♀️

It might be that we are doing something unusually weird with the dependency injection system in our app, but I couldn't find a clean way to fix the issue without this change in FastAPI.

Any idea if this has a chance to get merged soon? I would love to not have an internal fork of FastAPI anymore 😄

Rebased to resolve the conflicts

Merging this PR will not alter performance

✅ 24 untouched benchmarks

Comparing xkabylgSICKAG:master (e2775a9) with master (8e89896)1

No successful run was found on master (c3f316b) during the generation of this report, so 8e89896 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Looking forward for this fix since this improves the performance of our apps in production

Suggested change

      
                    if sub_dependant.use_cache:

Maybe makes sense (not sure actually, we need to think if it can break some specific use cases), but I think this is out of scope for this PR

This PR moves the check for key in dependency_cache into block above with continue, so check in this particular line will always be true.
Also there is no need to update dependency cache if dependency does not require caching in the first place.

If there is another call of the same dependency that would use the cache, then it wouldn't find the cached value from this call? (I'm not sure if that is what I would expect to happen, but it is the current behavior.)

Imagine the following:

async def needy_dependency(
    fresh_value: Annotated[str, Depends(get_value, use_cache=False)],
    cached_value: Annotated[str, Depends(get_value, use_cache=True)],
):

Previously fresh_value would be resolved and then added to the cache, and cached_value would be grabbed from the cache. With the suggested change it would not add the result from fresh_value to the cache and have to resolve cached_value.

… cached

Rebased on latest upstream and moved the cache_miss sentinel value outside the function as suggested by @dolfinus

xkabylgSICKAG force-pushed the master branch 2 times, most recently from 8bb7d6d to 69fca88 Compare March 20, 2024 17:57

YuriiMotov approved these changes Mar 20, 2024

View reviewed changes

Comment thread fastapi/dependencies/utils.py Outdated

xkabylgSICKAG force-pushed the master branch 3 times, most recently from c01036d to 1697c05 Compare March 21, 2024 13:03

estebanx64 added the enhancement label Apr 1, 2024

xkabylgSICKAG force-pushed the master branch from 1697c05 to e47c18b Compare October 18, 2024 08:38

xkabylgSICKAG force-pushed the master branch from e47c18b to 1e5edeb Compare November 19, 2024 14:55

xkabylgSICKAG force-pushed the master branch from 1e5edeb to 0bedb13 Compare February 10, 2025 08:49

svlandeg changed the title perf(dependencies): skip doing any work when sub dependant is already cached ⚡️ Skip doing any work when sub dependant is already cached Feb 18, 2025

svlandeg added feature New feature or request and removed enhancement labels Feb 18, 2025

This comment was marked as resolved.

Sign in to view

xkabylgSICKAG force-pushed the master branch 2 times, most recently from 9fdb058 to 0821c80 Compare February 19, 2025 21:55

svlandeg self-assigned this Feb 20, 2025

svlandeg reviewed Mar 28, 2025

View reviewed changes

Comment thread fastapi/dependencies/utils.py

svlandeg mentioned this pull request Mar 28, 2025

🐛 Fix issue with double model parsing #4089

Closed

This comment was marked as resolved.

Sign in to view

YuriiMotov reviewed May 14, 2025

View reviewed changes

xkabylgSICKAG force-pushed the master branch from dedc351 to 75d73d0 Compare May 19, 2025 05:40

svlandeg removed their assignment Jun 23, 2025

YuriiMotov approved these changes Jul 22, 2025

View reviewed changes

github-actions Bot added the conflicts Automatically generated when a PR has a merge conflict label Nov 3, 2025

This comment was marked as resolved.

Sign in to view

xkabylgSICKAG force-pushed the master branch from 7ca832e to 20460d3 Compare November 3, 2025 15:10

github-actions Bot removed the conflicts Automatically generated when a PR has a merge conflict label Nov 3, 2025

xkabylgSICKAG force-pushed the master branch from 20460d3 to 0264bcb Compare November 3, 2025 15:28

This comment was marked as off-topic.

Sign in to view

github-actions Bot added the conflicts Automatically generated when a PR has a merge conflict label Jul 24, 2026

This comment was marked as outdated.

Sign in to view

github-actions Bot removed the conflicts Automatically generated when a PR has a merge conflict label Jul 27, 2026

dolfinus reviewed Jul 28, 2026

View reviewed changes

Comment thread fastapi/dependencies/utils.py Outdated

dolfinus reviewed Jul 28, 2026

View reviewed changes

xkabylgSICKAG added 2 commits August 19, 2026 13:12

test(dependency_cache): add test for repeated parsing

de861d1

perf(dependencies): skip doing any work when sub dependant is already…

0627c68

… cached

xkabylgSICKAG force-pushed the master branch from d19dc29 to 0627c68 Compare August 19, 2026 11:34

🎨 Auto format

e2775a9

                           solved = await run_in_threadpool(call, **solved_result.values)
                       if sub_dependant.name is not None:
                           values[sub_dependant.name] = solved
                       if sub_dependant_cache_key not in dependency_cache:

⚡️ Skip doing any work when sub dependant is already cached by xkabylgSICKAG · Pull Request #11323 · fastapi/fastapi · GitHub

Uh oh!

Uh oh!

⚡️ Skip doing any work when sub dependant is already cached - #11323

⚡️ Skip doing any work when sub dependant is already cached#11323xkabylgSICKAG wants to merge 3 commits intofastapi:masterfastapi/fastapi:masterfrom xkabylgSICKAG:masterxkabylgSICKAG/fastapi:masterCopy head branch name to clipboard

Conversation

xkabylgSICKAG commented Mar 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YuriiMotov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xkabylgSICKAG commented Mar 21, 2024

Uh oh!

xkabylgSICKAG commented Mar 21, 2024

Uh oh!

This comment was marked as resolved.

xkabylgSICKAG commented Feb 18, 2025

Uh oh!

xkabylgSICKAG commented Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

YuriiMotov May 14, 2025

Choose a reason for hiding this comment

Uh oh!

dolfinus May 14, 2025

Choose a reason for hiding this comment

Uh oh!

YuriiMotov May 14, 2025

Choose a reason for hiding this comment

Uh oh!

xkabylgSICKAG May 14, 2025

Choose a reason for hiding this comment

Uh oh!

YuriiMotov left a comment

Choose a reason for hiding this comment

Uh oh!

xkabylgSICKAG commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

xkabylgSICKAG commented Nov 3, 2025

Uh oh!

codspeed-hq Bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

ranjanprasad96 commented Jun 7, 2026

Uh oh!

This comment was marked as off-topic.

This comment was marked as outdated.

Uh oh!

dolfinus Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

YuriiMotov Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

dolfinus Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xkabylgSICKAG Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

xkabylgSICKAG commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

⚡️ Skip doing any work when sub dependant is already cached#11323
xkabylgSICKAG wants to merge 3 commits into
fastapi:masterfrom
xkabylgSICKAG:master

xkabylgSICKAG commented Mar 20, 2024 •
edited
Loading

YuriiMotov left a comment •
edited
Loading

xkabylgSICKAG commented Feb 19, 2025 •
edited
Loading

xkabylgSICKAG commented Aug 4, 2025 •
edited
Loading

codspeed-hq Bot commented Jan 8, 2026 •
edited
Loading

dolfinus Jul 28, 2026 •
edited
Loading