search

pydantic / pydantic-core

Core validation logic for pydantic written in rust
MIT License
1.44k stars 247 forks source link

Simplify shared union serializer logic #1538

Closed sydney-runkle closed 1 week ago

codspeed-hq[bot] commented 2 weeks ago

CodSpeed Performance Report

Merging #1538 will not alter performance

Comparing dh/wip-union-simplification (ee9dd3d) with main (061711f)

Summary

✅ 155 untouched benchmarks

sydney-runkle commented 1 week ago

Failing integration test is unrelated to this PR, should be resolved soon once we merge https://github.com/pydantic/pydantic/pull/10825 into main on pydantic

MarkusSintonen commented 1 week ago

@sydney-runkle did you try running CodSpeed in Pydantic side for this PR? I accidentally rebased my changes on top of pydantic-core master (instead of release tag) and pointed Pydantic side PR to point into the master based code (which included only this PR changes). CodSpeed showed this which is quite a regression:

❌ test_north_star_dump_json 31.7 ms 57.9 ms -45.26%

davidhewitt commented 1 week ago

I think the benchmarks we have here should have identified if this PR was a problem, but it's not impossible it could slip through.

davidhewitt commented 1 week ago

This PR should in theory have no functional change so definitely that order of magnitude seems incorrect.

MarkusSintonen commented 1 week ago

@davidhewitt / @sydney-runkle it is still showing the exactly same here: https://github.com/pydantic/pydantic/pull/10845 🤔 Weird...

sydney-runkle commented 1 week ago

Huh, that is really odd. @davidhewitt, I might defer to you here - not sure why our changes here would degrade perf...