Skip to content

Conversation

@davidhewitt
Copy link
Contributor

@davidhewitt davidhewitt commented Sep 29, 2025

Change Summary

This is an experiment to see what happens if definition inlining is implemented inside SchemaValidator. This should avoid the need to do it in Python (in the clean_schema function).

Hoping this has performance benefits.

Not fully implemented yet, there's some test diffs due to changes in the recursion defenses, I think I know how to resolve.

For now I haven't particularly changed the Python code, just made GenerateSchema not bother running cleaning if there are no deferred discriminators to apply.

Related issue number

N/A

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Sep 29, 2025
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 29, 2025

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5da3807
Status: ✅  Deploy successful!
Preview URL: https://5842a178.pydantic-docs.pages.dev
Branch Preview URL: https://dh-core-inline-definitions.pydantic-docs.pages.dev

View logs

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 30, 2025

CodSpeed Performance Report

Merging #12294 will degrade performances by 5.67%

Comparing dh/core-inline-definitions (0131626) with main (58befdc)

Summary

⚡ 12 improvements
❌ 1 regression
✅ 33 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
test_fastapi_startup_perf 82.9 ms 76.7 ms +8.12%
test_fastapi_startup_perf 19.5 ms 18.4 ms +6.05%
test_construct_dataclass_schema 1.1 ms 1 ms +6.64%
test_model_validators_serializers 779.8 µs 735.9 µs +5.96%
test_nested_model_schema_generation 818.9 µs 754.4 µs +8.54%
test_recursive_model_schema_generation 835.5 µs 791.4 µs +5.57%
test_tagged_union_with_callable_discriminator_schema_generation 1,046.5 µs 952.9 µs +9.82%
test_tagged_union_with_str_discriminator_schema_generation 1,039.5 µs 950.5 µs +9.36%
test_generic_recursive_model_schema_generation 735.9 µs 694 µs +6.04%
test_nested_recursive_generic_model_schema_generation 1.4 ms 1.3 ms +5.33%
test_nested_recursive_model_schema_generation 1.5 ms 1.4 ms +5.08%
test_simple_recursive_model_schema_generation 614.8 µs 573.1 µs +7.27%
test_list_of_models_serialization 58.5 µs 62 µs -5.67%

@davidhewitt
Copy link
Contributor Author

cc @Viicos this implements a first step in what I proposed by moving schema inlining to Rust. It seems like this is currently a decent win ~10%, however it could change a lot depending on what we can do with schema caching and config schema etc

@davidhewitt
Copy link
Contributor Author

(serialization slowdown is because serializers don't inline in Rust yet on the draft core branch, also a few small test failures is not a surprise, these are mostly from asserting the schema type, which has changed, and also from a few changes to recursion handling which I think I can trivially prevent in core)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotes-fix Used for bugfixes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants