Use Simdjson ondemand parser instead of DOM parser #3878
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Klaim
added
the
release::enhancements
JohanMabille
approved these changes
Apr 9, 2025
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3878 +/- ##
=======================================
Coverage ? 62.72%
=======================================
Files ? 299
Lines ? 36721
Branches ? 2756
=======================================
Hits ? 23033
Misses ? 13636
Partials ? 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AntoinePrv
approved these changes
Apr 10, 2025
first non-working "naive" conversion, compiles but never ends and allocates more than 11Gb of memory
…ingthe info field in json properly
Klaim
force-pushed
the
simdjson-ondemand
branch
from
8d32c21 to
3479ad2
Compare
jjerphan
approved these changes
Apr 17, 2025
Comment on lines
+551
to
+560
| // BEWARE: | ||
| // We use below `simdjson`'s "on-demand" parser, which does not tolerate reading the same | ||
| // value more than once. This means we need to make sure that the objects and their fields | ||
| // are read and/or concretized only once and if we need to use them more than once we need | ||
| // to persist them in local memory. This is why the code below tries hard to pre-read the | ||
| // data needed in several parts of the computing in a way that prevents jumping up and down | ||
| // the hierarchy of json objects. When this rule is not followed, the parsing might end | ||
| // earlier than expected or might skip data that are read when they shouldn't be, leading to | ||
| // *runtime issues* that might not be visible at first. Because of these reasons, be careful | ||
| // when modifying the following parsing code. |
There was a problem hiding this comment.
Thank you for mentioning this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Running
main'stest_libmambaexecutable currently allocates (but dont use) more than 5Gb when parsing 320Mb sized JSON files. This PR reduces this to around 2.5Gb. (observed using Visual Studio, in both Release and RelWithDebugInfo, also got some confirmation that this is observable on Linux)flat_set<T>to allow.contains(value)wherevaluehas a different type thanTbut is still comparable.