Skip to content
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

Decent optimizations for CssAttributeCollection #322

Conversation

patrikwlund
Copy link
Contributor

Changes

The core change is that we don't try to be a dictionary anymore, just a simple list. No need for the CssValue struct either, just store the CssAttribute directly in the list. The order of the list is the same as the attribute order, so no need to sort it every time we want to iterate over it.

Because this is a public type, these are probably considered breaking changes.

Realistic benchmark of MoveCssInline()

One invocation of MoveCssInline()
Original: 1.2ms 592kB
After: 1.0ms 482kB

@martinnormark
Copy link
Contributor

Thanks for the PR!

Do you have the example CSS you used for benchmarking? I'm curious to see how performance scales with attribute key/style lookups.

@patrikwlund
Copy link
Contributor Author

Hi @martinnormark, I created another PR with a benchmark that can be used to bench this #323

Original

Method Mean Error StdDev Median Gen0 Allocated
AngleSharpBaseline 170.6 us 6.79 us 20.03 us 164.9 us - 138.67 KB
MoveCssInline 832.4 us 5.43 us 14.69 us 830.9 us 30.0000 621.52 KB

After this PR

Method Mean Error StdDev Median Gen0 Allocated
AngleSharpBaseline 174.3 us 9.63 us 28.40 us 162.9 us - 138.67 KB
MoveCssInline 704.5 us 2.73 us 7.33 us 703.8 us 30.0000 506.61 KB

Realistic benchmark of MoveCssInline()
Original: 1.2ms 592kB
After:  1.0ms 482kB
Copy link
Contributor

@martinnormark martinnormark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement! I might expand on the benchmarking with more automation and more example email HTML, to focus on performance of internals like this.

@martinnormark martinnormark merged commit 60f1eb5 into milkshakesoftware:master Jan 1, 2023
@patrikwlund patrikwlund deleted the optimize-CssAttributeCollection branch January 1, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants