Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix config.defer_build for serialization first cases #7024

Merged
merged 8 commits into from Aug 23, 2023

Conversation

samuelcolvin
Copy link
Member

@samuelcolvin samuelcolvin commented Aug 7, 2023

Change Summary

@MarkusSintonen might be a good idea if you try this as a PR, rather than waiting a release to find my dumb bugs 🙃 ?

Related issue number

continues from #6823 as per comment from @MarkusSintonen #6768 (comment)

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

Selected Reviewer: @dmontagu

@cloudflare-pages
Copy link

cloudflare-pages bot commented Aug 7, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1977786
Status: ✅  Deploy successful!
Preview URL: https://c62fc1e7.pydantic-docs2.pages.dev
Branch Preview URL: https://defer-build-on-serialization.pydantic-docs2.pages.dev

View logs

@samuelcolvin
Copy link
Member Author

just the fastapi tests failing. Please review.

@samuelcolvin
Copy link
Member Author

@MarkusSintonen if you do have a chance to look at this, another cprofile output like you added to #6768 would be really useful 🙏.

@MarkusSintonen
Copy link
Contributor

MarkusSintonen commented Aug 8, 2023

@samuelcolvin testing with this branch still gives some strange failures when enabling defer_build ("globally"). I was able to isolate the issue a bit (how it actually happens on our codebase is slightly more complex):

def test_partial_creation_with_defer_build():
    BaseModel.model_config["defer_build"] = True  # Actually setup close to the (tests) entrypoint. Feels a bit hacky.

    class M(BaseModel):
        a: int
        b: int

    def create_partial(model: type[BaseModel], optionals: set[str]) -> type[BaseModel]:
        override_fields: dict[str, tuple[Any, FieldInfo]] = {}
        for name, field in model.model_fields.items():
            if field.is_required() and name in optionals:
                assert field.annotation is not None
                override_fields[name] = ((field.annotation | None), FieldInfo.merge_field_infos(field, default=None))

        return create_model(f"Partial{model.__name__}", __base__=model, **cast(dict, override_fields))

    partial = create_partial(M, {"a"})

    # Comment this away and the last assertion works
    assert M.model_json_schema()["required"] == ["a", "b"]

    # AssertionError: assert ['a', 'b'] == ['b']
    assert partial.model_json_schema()["required"] == ["b"]

@MarkusSintonen
Copy link
Contributor

cProfile improves a bit for FastAPI app init. So there is maybe 10-20% decrease in the amount of "walking". Would have maybe expected it to improve more 🤔

With defer_build-on-serialization branch + BaseModel.model_config["defer_build"] = True:

3429857/370439    4.131    0.000   16.896    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:193(_walk)
3423324/370439    3.487    0.000   18.429    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:190(walk)
    10618    2.761    0.000   60.892    0.006 xxx/python3.11/site-packages/pydantic/type_adapter.py:144(__init__)
   222090    2.306    0.000    8.959    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:1490(_get_prepare_pydantic_annotations_for_known_type)
120522/28654    2.006    0.000   42.288    0.001 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:873(_common_field_schema)
  3373638    1.781    0.000    2.618    0.000 {built-in method builtins.getattr}
1361941/179166    1.713    0.000    6.978    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:434(flatten_refs)
8266030/7897562    1.663    0.000    4.272    0.000 {built-in method builtins.isinstance}
295501/253432    1.424    0.000   11.487    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:309(handle_model_fields_schema)
134777/14051    1.263    0.000   48.091    0.003 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:1530(_apply_annotations)
  5221553    1.261    0.000    1.261    0.000 {method 'copy' of 'dict' objects}
 10006010    1.208    0.000    1.208    0.000 {method 'get' of 'dict' objects}
2852217/604895    1.150    0.000   14.618    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:200(_handle_other_schemas)
979112/54744    1.126    0.000    5.337    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:411(collect_refs)
201209/45264    1.123    0.000   41.375    0.001 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:573(_generate_schema_from_property)
   101836    1.033    0.000    1.448    0.000 xxx/python3.11/typing.py:1896(_get_protocol_attrs)
163922/11607    1.012    0.000   48.694    0.004 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:661(_generate_schema)
  2022527    1.003    0.000    1.431    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:320(_config_wrapper)
  2765577    0.985    0.000    1.300    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:106(get_ref)
   243410    0.982    0.000    2.371    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:69(get_type_ref)
   827691    0.880    0.000    1.321    0.000 xxx/python3.11/typing.py:2402(get_origin)
32934/4089    0.831    0.000   39.995    0.010 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:459(_model_schema)
   432145    0.814    0.000    3.627    0.000 xxx/python3.11/site-packages/pydantic/_internal/_known_annotated_metadata.py:41(expand_grouped_metadata)
    73498    0.765    0.000    2.228    0.000 xxx/python3.11/inspect.py:2333(_signature_from_function)
122455/73498    0.748    0.000    3.891    0.000 xxx/python3.11/inspect.py:2428(_signature_from_callable)
   599005    0.679    0.000    0.679    0.000 xxx/python3.11/site-packages/pydantic_core/core_schema.py:3914(<dictcomp>)
   245405    0.633    0.000    3.160    0.000 xxx/python3.11/site-packages/pydantic/_internal/_known_annotated_metadata.py:180(collect_known_metadata)
120368/56415    0.629    0.000   24.223    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:686(match_type)
   599005    0.607    0.000    1.380    0.000 xxx/python3.11/site-packages/pydantic_core/core_schema.py:3913(_dict_not_none)
   478350    0.590    0.000    3.029    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:2011(get_schema_or_ref)
329633/10816    0.572    0.000    1.619    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:465(count_refs)
   163437    0.552    0.000    1.225    0.000 xxx/python3.11/site-packages/pydantic/_migration.py:262(wrapper)
   279613    0.523    0.000    0.593    0.000 xxx/python3.11/contextlib.py:104(__init__)
194993/120524    0.514    0.000    2.664    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generics.py:341(has_instance_in_type)
789931/788833    0.512    0.000    0.768    0.000 {built-in method builtins.hasattr}
   134196    0.495    0.000    0.691    0.000 xxx/python3.11/inspect.py:2972(__init__)
176025/175869    0.485    0.000    0.485    0.000 xxx/python3.11/site-packages/ddtrace/internal/module.py:342(__getattribute__)
572764/571084    0.475    0.000    3.612    0.000 {built-in method builtins.next}
    49573    0.455    0.000    0.722    0.000 xxx/python3.11/site-packages/pydantic/_internal/_config.py:127(core_config)
   185694    0.453    0.000    0.772    0.000 xxx/python3.11/inspect.py:2686(__init__)
  2022527    0.428    0.000    0.428    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:268(tail)
357351/11607    0.425    0.000    1.725    0.000 xxx/python3.11/site-packages/pydantic/_internal/_discriminated_union.py:24(inner)
   210059    0.417    0.000    2.277    0.000 xxx/python3.11/site-packages/pydantic/_internal/_std_types_schema.py:345(datetime_prepare_pydantic_annotations)
  1766161    0.416    0.000    0.416    0.000 {method 'startswith' of 'str' objects}
   153456    0.415    0.000    0.735    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_metadata.py:65(build_metadata_dict)
279938/214467    0.385    0.000    0.404    0.000 {built-in method _abc._abc_subclasscheck}
115384/14743    0.371    0.000   43.916    0.003 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:1562(inner_handler)
   362170    0.367    0.000    0.367    0.000 xxx/python3.11/site-packages/pydantic/_internal/_config.py:118(__getattr__)
   222090    0.363    0.000    1.055    0.000 xxx/python3.11/site-packages/pydantic/_internal/_std_types_schema.py:552(sequence_like_prepare_pydantic_annotations)
   482088    0.346    0.000    0.515    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:160(filter_field_decorator_info_by_field)

2.1.1 without defer_build

3864438/399988    4.726    0.000   21.174    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:190(walk)
3868371/399988    4.285    0.000   19.578    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:193(_walk)
    10618    2.760    0.000   35.949    0.003 xxx/python3.11/site-packages/pydantic/type_adapter.py:144(__init__)
1357448/180123    1.760    0.000    7.654    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:434(flatten_refs)
308355/255280    1.640    0.000   13.560    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:309(handle_model_fields_schema)
1151300/40205    1.556    0.000    6.916    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:411(collect_refs)
3239921/684268    1.481    0.000   17.087    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:200(_handle_other_schemas)
  5792260    1.403    0.000    1.403    0.000 {method 'copy' of 'dict' objects}
  9844232    1.139    0.000    1.139    0.000 {method 'get' of 'dict' objects}
  3015226    1.086    0.000    1.422    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:106(get_ref)
3917583/3767717    0.922    0.000    2.601    0.000 {built-in method builtins.isinstance}
    84934    0.858    0.000    3.878    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:1472(_get_prepare_pydantic_annotations_for_known_type)
    71703    0.727    0.000    1.013    0.000 xxx/python3.11/typing.py:1896(_get_protocol_attrs)
  1454419    0.705    0.000    0.875    0.000 {built-in method builtins.getattr}
42672/25664    0.701    0.000   16.275    0.001 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:878(_common_field_schema)
55071/14379    0.555    0.000   22.581    0.002 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:1512(_apply_annotations)
207175/137345    0.459    0.000    0.479    0.000 {built-in method _abc._abc_subclasscheck}
404504/11571    0.442    0.000    2.044    0.000 xxx/python3.11/site-packages/pydantic/_internal/_discriminated_union.py:24(inner)
75090/35330    0.422    0.000   15.092    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:581(_generate_schema_from_property)
   359160    0.411    0.000    0.628    0.000 xxx/python3.11/typing.py:2402(get_origin)
      953    0.409    0.000    8.815    0.009 xxx/python3.11/site-packages/pydantic/_internal/_model_construction.py:418(complete_model_class)
407497/11356    0.408    0.000    1.711    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:465(count_refs)
    89302    0.406    0.000    0.820    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:69(get_type_ref)
404504/11571    0.379    0.000    2.064    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:116(_record_valid_refs)
    32706    0.371    0.000    1.080    0.000 xxx/python3.11/inspect.py:2333(_signature_from_function)
   188009    0.370    0.000    2.469    0.000 xxx/python3.11/site-packages/pydantic/_internal/_known_annotated_metadata.py:41(expand_grouped_metadata)
55487/32706    0.364    0.000    1.894    0.000 xxx/python3.11/inspect.py:2428(_signature_from_callable)
   758965    0.346    0.000    0.510    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:319(_config_wrapper)
   300746    0.322    0.000    0.681    0.000 xxx/python3.11/site-packages/pydantic_core/core_schema.py:3913(_dict_not_none)
    35564    0.322    0.000    0.488    0.000 xxx/python3.11/copy.py:259(_reconstruct)
   300746    0.314    0.000    0.314    0.000 xxx/python3.11/site-packages/pydantic_core/core_schema.py:3914(<dictcomp>)
52914/10205    0.313    0.000   18.288    0.002 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:669(_generate_schema)
    96041    0.288    0.000    1.807    0.000 xxx/python3.11/site-packages/pydantic/_internal/_known_annotated_metadata.py:180(collect_known_metadata)
  1357448    0.268    0.000    0.268    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:39(is_definitions_schema)
   269123    0.260    0.000   22.311    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:384(walk_core_schema)
    40205    0.260    0.000   18.045    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:401(_simplify_schema_references)
409864/408766    0.260    0.000    0.347    0.000 {built-in method builtins.hasattr}
    67768    0.253    0.000    0.355    0.000 xxx/python3.11/inspect.py:2972(__init__)
   105026    0.253    0.000    0.439    0.000 xxx/python3.11/inspect.py:2686(__init__)
  1138977    0.251    0.000    0.251    0.000 {method 'startswith' of 'str' objects}
44896/24673    0.227    0.000    7.660    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:691(match_type)
299510/11356    0.227    0.000    1.079    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:483(inline_refs)
   174648    0.224    0.000    1.056    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:1991(get_schema_or_ref)
    23805    0.223    0.000    0.349    0.000 xxx/python3.11/site-packages/pydantic/_internal/_config.py:127(core_config)
    62109    0.215    0.000    0.481    0.000 xxx/python3.11/site-packages/pydantic/_migration.py:262(wrapper)
75105/42672    0.196    0.000    0.985    0.000 xxx/python3.11/site-packages/pydantic/_internal/_generics.py:341(has_instance_in_type)
8018/3035    0.195    0.000   14.150    0.005 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:470(_model_schema)
65720/65564    0.186    0.000    0.186    0.000 xxx/python3.11/site-packages/ddtrace/internal/module.py:342(__getattribute__)
46180/13996    0.184    0.000   18.740    0.001 xxx/python3.11/site-packages/pydantic/_internal/_generate_schema.py:1544(inner_handler)
    42083    0.176    0.000    0.861    0.000 xxx/python3.11/site-packages/pydantic/_internal/_core_utils.py:345(handle_dataclass_args_schema)

@samuelcolvin
Copy link
Member Author

This is really useful, thank you.

Would have maybe expected it to improve more 🤔

Hoped, but not expected. 🤷

We have some big plans in the works to remove some calls to walk, in the meantime we'll continue to look for smaller improvements.

@hramezani
Copy link
Member

Needs to be refreshed
Please update

@samuelcolvin
Copy link
Member Author

@samuelcolvin testing with this branch still gives some strange failures when enabling defer_build ("globally"). I was able to isolate the issue a bit (how it actually happens on our codebase is slightly more complex):

def test_partial_creation_with_defer_build():
    BaseModel.model_config["defer_build"] = True  # Actually setup close to the (tests) entrypoint. Feels a bit hacky.

    class M(BaseModel):
        a: int
        b: int

    def create_partial(model: type[BaseModel], optionals: set[str]) -> type[BaseModel]:
        override_fields: dict[str, tuple[Any, FieldInfo]] = {}
        for name, field in model.model_fields.items():
            if field.is_required() and name in optionals:
                assert field.annotation is not None
                override_fields[name] = ((field.annotation | None), FieldInfo.merge_field_infos(field, default=None))

        return create_model(f"Partial{model.__name__}", __base__=model, **cast(dict, override_fields))

    partial = create_partial(M, {"a"})

    # Comment this away and the last assertion works
    assert M.model_json_schema()["required"] == ["a", "b"]

    # AssertionError: assert ['a', 'b'] == ['b']
    assert partial.model_json_schema()["required"] == ["b"]

Thanks so much @MarkusSintonen I've added your tests to our unit tests, unfortunately since Model.model_fields is a class attribute it's not trivial to convert it to a property and call model_rebuild() within it - I worried that the solutions I tried will have edge cases that will break existing code.

In summary, you'll need to call model_rebuild() manually before accessing Model.model_fields if you're using defer_build=True.

@samuelcolvin
Copy link
Member Author

please review.

@adriangb adriangb merged commit bd2b524 into main Aug 23, 2023
50 checks passed
@adriangb adriangb deleted the defer_build-on-serialization branch August 23, 2023 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants