Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Schema cleaning: skip unnecessary copies during schema walking#10286

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sydney-runkle merged 3 commits intomainfromschema-cleaning-no-copy
Sep 27, 2024

Conversation

Viicos
Copy link
Member

@ViicosViicos commentedSep 2, 2024
edited
Loading

Related issue:

A newcopy argument is added to_WalkCoreSchema, defaulting toTrue. In the relevant steps of schema cleaning, no copy is performed.

Flamegraph:

Time order:

image

Left heavy:

image


The left heavy schema clearly shows thatcollect_refs (the first handler function to be used withwalk_core_schema) takes most of the time, and the other ones now represents almost nothing.

Seems like ~1 second can be saved. Not bad, but not that great either. Perhaps dict copies aren't that expensive? Unsurprisingly, memory consumption is the same (~600MiB in the end):

  • Command:memray run -o k8s_v2_no_copy.bin k8s_v2.py && memray flamegraph k8s_v2_no_copy.bin

image

@github-actionsgithub-actionsbot added the relnotes-fixUsed for bugfixes. labelSep 2, 2024
@ViicosViicosforce-pushed theschema-cleaning-no-copy branch fromf519d94 to80e1de2CompareSeptember 2, 2024 16:04
@cloudflare-workers-and-pagesCloudflare Workers and Pages
Copy link

cloudflare-workers-and-pagesbot commentedSep 2, 2024
edited
Loading

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit:7967d58
Status: ✅  Deploy successful!
Preview URL:https://aa61922c.pydantic-docs.pages.dev
Branch Preview URL:https://schema-cleaning-no-copy.pydantic-docs.pages.dev

View logs

@codspeed-hqCodSpeed HQ
Copy link

codspeed-hqbot commentedSep 2, 2024
edited
Loading

CodSpeed Performance Report

Merging#10286 willnot alter performance

Comparingschema-cleaning-no-copy (7967d58) withmain (8fe7c8e)

Summary

✅ 38 untouched benchmarks

@ViicosViicosforce-pushed theschema-cleaning-no-copy branch from80e1de2 to9d4745cCompareSeptember 2, 2024 16:10
@github-actionsGitHub Actions
Copy link
Contributor

github-actionsbot commentedSep 4, 2024
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic/_internal
  _core_utils.py
  _discriminated_union.py
Project Total 

This report was generated bypython-coverage-comment-action

@sydney-runkle
Copy link
Contributor

@Viicos is this ready for review?

@sydney-runkle
Copy link
Contributor

@Viicos,

Could you add a note to this PR re incompatibility with a cache based approach? I'm keen to accept this as is for now, then revert if we come up with a good caching solution pre our next release.

@ViicosViicos marked this pull request as ready for reviewSeptember 27, 2024 13:50
Copy link
Contributor

@sydney-runklesydney-runkle left a comment

Choose a reason for hiding this comment

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

This looks good to me.

As mentioned on the schema walking performance analysis issue, I think there are more improvements to be made here, but this is a good start.

Thanks@Viicos!

@sydney-runklesydney-runkle added relnotes-performanceUsed for performance improvements. and removed relnotes-fixUsed for bugfixes. labelsSep 27, 2024
@sydney-runklesydney-runkle merged commit22ba7fa intomainSep 27, 2024
61 checks passed
@sydney-runklesydney-runkle deleted the schema-cleaning-no-copy branchSeptember 27, 2024 18:12
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@sydney-runklesydney-runklesydney-runkle approved these changes

Assignees
No one assigned
Labels
relnotes-performanceUsed for performance improvements.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@Viicos@sydney-runkle

[8]ページ先頭

©2009-2025 Movatter.jp