-
Notifications
You must be signed in to change notification settings - Fork 236
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
Update maybe_xml in compare
project to the latest version and add quick-xml borrowed to compare table
#691
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #691 +/- ##
=======================================
Coverage 65.06% 65.06%
=======================================
Files 38 38
Lines 17942 17942
=======================================
Hits 11674 11674
Misses 6268 6268
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
How much was it reduced, and have you done any profiling? |
Not much actually. After the change we win in couple tests, but in other we still 1.5x-2x slower that maybe_xml. I did not ran the profiler yet (I need to find the right tool and how to correctly interpret the results, my experience in profiling is almost zero). I think about two possible reasons:
|
quick-xml was benchmarked in buffered mode which a bit unfair
I forget that you use Windows, it's definitely more challenging on Windows than Linux. If there are any samples you'd like to see profiled I can run them for you. I have a few real-world cases in mind that I could test out. |
d59f39f
to
82cc0da
Compare
Right now I'm not know from where I should start. I tried to benchmark the new parser and unfortunately it is always 2x slower that current implementation. I expected an improvement or at least the same results. Need to figure out why it so slow, because the new implementation is definitely simpler to understand. It will be great to benchmark and profile the |
Well, just checked -- |
I found, that at least part of our slowdown against maybe_xml is because we benchmarked in buffered mode, while maybe_xml completely allocation-free. Add the same variant for quick-xml