⚡ Update create_cloned_field to use a global cache and improve startup performance

[zanieb]

Resolves issue reported in #4644 by instantiating a global dictionary to store cloned fields. This prevents repeated copying of models which can have significant performance affects. Please see the issue for more details.

This uses a WeakKeyDictionary so we avoid holding a reference to the model. I doubt this will have much practical affect since response model types are often globally defined, but it seems like good practice. This should not increase memory overhead since all the models generated by create_cloned_field will be in scope for the scope of the FastAPI application anyway — users have noted this can significantly reduce memory consumption as it does not create as many model copies.

It assigns the dictionary as a default value instead of moving the parameter to an explicitly global value. This allows a new cache to be passed if desired. However, mutable default values can be confusing and it would be fair to move it out of the function signature. edit: this change was made to meet test coverage.

#4644 includes profiling indicating the performance increases here.

As mentioned in the issue, #894 (comment) refers to an alternative approach that would require work in Pydantic that would make this function irrelevant. That seems better in the long, but this is an easier patch for now.

See also discussion at #8609
Related to #4644

[ddanier]

Does not close #4644 - see comment in the linked issue.

[orfisko]

Isn't this a similar solution as what has been proposed here? #4105

[zanieb]

@orfisko Yeah it is! I missed that implementation since it wasn't linked to an issue. The choices here are a little different:

• The cloned_types parameter is retained. This prevents a breaking change to the function signature and allows a caller to opt-out of the cache or provide a different one.
• A WeakKeyDictionary is used. This is mostly a matter of best-practice preventing a cache from creating strong references to types. This could prevent increased memory usage if types are defined dynamically. I doubt it'll have much practical effect though.

[zanieb]

Does not close #4644 - see comment in the linked issue.

For others who read this, this comment refers specifically to an issue with using dynamically generated models with ForwardRef fields with this patch. Even then, you can work around the problem by updating forward refs. If someone provides an MRE, I can look into resolving the issue. This does fix the memory consumption and initialization times, see the update from the same user in #4644 (comment).

[github-actions]

📝 Docs preview for commit 4d7d18e at: https://636aa2502be5da007c6dc54e--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit d741490 at: https://636aa752e169f6006f8afab1--fastapi.netlify.app

[michaelgmiller1]

This is a great addition and sorely needed for the application I'm working on. Please let me know if there's anything I can do to help push this over the finish line!

[exherb]

when is this going to merge and release?

[zanieb]

Note that coverage is failing because this utility is not explicitly tested and consequently there's not coverage for actually using a custom cloned_types value.

@tiangolo The CI failures at https://github.com/tiangolo/fastapi/actions/runs/3535147825/jobs/5932836062 appear to be related to an change in GitHub actions that breaks Python when using actions/cache@v3. We encountered this at Prefect as well, resetting the cache resolved the issue. See #5680.

[github-actions]

📝 Docs preview for commit 6ab34f2 at: https://637e77005289633eb24dbfd5--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit b64052f at: https://638366a8319a3972152f2ec9--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit f378dfa at: https://63837048cb2392708ef8aca8--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 076f15a at: https://639ce7f5ecf1fd09839b403e--fastapi.netlify.app

[michaelgmiller1]

Is this ready to ship? Anything I can do to help push this across the finish line?

[zanieb]

This is ready to ship although it does not currently meet coverage requirements for the project. The maintainer has marked this as an issue to investigate when they have time.

[michaelgmiller1]

@tiangolo any chance we could get this merged, if it's ready to go? this is a huge win, making both app startup and test startup significantly faster.

[github-actions]

📝 Docs preview for commit bf5c338 at: https://63fcf8ba6775e90057a15a43--fastapi.netlify.app

[Lawouach]

Indeed @tiangolo. This would be a nice win all around :)

[github-actions]

📝 Docs preview for commit d8bea1e at: https://643b6e9ed45d4c5bfe2bf01b--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit ac82725 at: https://645507701034401069366570--fastapi.netlify.app

[acookin]

@tiangolo What's required to get this merged?

[github-actions]

📝 Docs preview for commit 7184259 at: https://645ea712446d774207e234db--fastapi.netlify.app

[followben]

Thanks for keeping this up to date @madkinsz. Personally I think this is an improvement over the proposal in #4105 and would be my preference. @tiangolo, sorry to bug but can you tell shed light on what's holding this up?

[github-actions]

📝 Docs preview for commit 730717c at: https://646f7e9ccc99ff0466e10339--fastapi.netlify.app

[ulan-yisaev]

Hey team, looking forward for this merge :)

[tiangolo]

Awesome, thanks a lot @madkinsz! 🙇

Thanks everyone for the comments, sorry for the long delay. 😅 🙈

I've been working on the migration to Pydantic v2, which will make this clone_field logic unnecessary altogether, as everything will be done on the Pydantic-core (Rust) side. Nevertheless, I'll first make a release with this improvement.

This will be available in FastAPI 0.96.0, released in the next hours. 🚀

---

@huonw I'm making you an honorary co-author of this PR for your work on #4105 that was before this. I'm taking this one just for the simplification in the changes/diff, the weakref, and keeping the signature. 🤓