-
-
Notifications
You must be signed in to change notification settings - Fork 49
remove SmallVec and LazyIndexMap from json value
#184
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@ ## main #184 +/- ## ========================================== - Coverage 88.71% 88.36% -0.36% ========================================== Files 13 12 -1 Lines 2207 2106 -101 Branches 2207 2106 -101 ========================================== - Hits 1958 1861 -97 + Misses 153 151 -2 + Partials 96 94 -2
Continue to review full report in Codecov by Sentry.
|
CodSpeed Performance ReportMerging #184 will improve performances by 11.14%Comparing Summary
Benchmarks breakdown
|
@samuelcolvin I have tested this all the way up the stack e.g. in pydantic/pydantic#11439
I am satisfied that the performance here is fine and we should merge this. In core we can do fancy iteration if we want, but they don't add much benefit and do add a lot of complexity so I am not rushing to merge that. (Eventually we might want the core changes if we move to jiter iteration in core.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Uh oh!
There was an error while loading. Please reload this page.
This simplifies the
jiterpublic API (particularly onJsonValue) by simplifying theJsonObjectandJsonArraytypes to just useArc<Vec<...>>rather thansmallvecin the array case andLazyIndexMapin the object case. This hasn't affected the way that we parse or the algorithmic complexity, it's a pure simplification.At the same time we completely remove
LazyIndexMapwhich attempted to be a thread-safe data structure which lazily constructed a mapping for iteration to guarantee uniqueness. I'm reasonably sure it became a "jack-of all trades, master of none" situation and there are bugs in it, maybe also performance costs.