Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Small optimizations #3

Closed
wants to merge 2 commits into from
Closed

Small optimizations #3

wants to merge 2 commits into from

Conversation

dralley
Copy link
Contributor

@dralley dralley commented May 2, 2022

Split out from the criteron benchmarks because it will need more manual rework to backport

commentary on tafia/quick-xml#348

11% improvement on iter_attributes bench
19% improvement on try_get_attribute bench
@Mingun
Copy link
Owner

Mingun commented May 3, 2022

Current implementation of attributes parsing have bugs on some corner cases, so I've rewrite it from scratch, so all this changes now irrelevant. So I'll close this PR, but thanks for the work!

@Mingun Mingun closed this May 3, 2022
@dralley
Copy link
Contributor Author

dralley commented May 3, 2022

@Mingun How about I add the benchmarks to this PR, then merge the optimizations anyway, and rebase changes on top? So that you can properly compare the performance of the two implementations.

@Mingun Mingun mentioned this pull request May 3, 2022
@Mingun
Copy link
Owner

Mingun commented May 3, 2022

There is no point in measuring the performance of incorrect code, because it will still be rewritten (already done: #4). But it makes sense to measure the performance of the new code and try to make the same optimizations. So I think it is better to open a new PR with benchmark changes, then merge it, rebase #4 over it and merge it (with optimizations if that will be valuable)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants