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

Added ElementInfo caching for faster SvgElement Creation #764

Merged
merged 4 commits into from
Jan 1, 2021

Conversation

inforithmics
Copy link
Contributor

@inforithmics inforithmics commented Oct 4, 2020

This causes the Performance Test to finish in 10 seconds instead of 22 seconds without the optimizations.

Reference Issue

#767

What does this implement/fix? Explain your changes.

I profiled a Mobile App that uses this svg library and the profiler showed that a large part of the parsing of Svg is spent in
creating the ElementInfo, I cache it statically, because it can't change during loading.

Any other comments?

Added an unit test for performance that times out after 25 seconds. Maybe the timeout can be adjusted downwards.
On my machine this Unit Test runs in 10 seconds.

@mrbean-bremen
Copy link
Member

@H1Gdev, anyone else interested: this and the other performance PR seem to be ready to be merged, is there anything preventing the merge from your perspective?

@wieslawsoltes
Copy link
Contributor

I am working on similar perf improvement in #772, Source Generators have advantage of being compile time operation, also remove usage of Activator.CreateInstance which should add even more performance.

@mrbean-bremen
Copy link
Member

I am working on similar perf improvement in #772

Would this replace this PR, or add to it?

@mrbean-bremen
Copy link
Member

@wieslawsoltes: if you are adding to it, it would make sense to merge it (and the other performance PR) now, what do you think?

@wieslawsoltes
Copy link
Contributor

@wieslawsoltes: if you are adding to it, it would make sense to merge it (and the other performance PR) now, what do you think?

You can merge this PR, I can port my changes, no problem.

@wieslawsoltes
Copy link
Contributor

I am working on similar perf improvement in #772

Would this replace this PR, or add to it?

I can add my changes after this PR

@mrbean-bremen mrbean-bremen merged commit 7c44bd5 into svg-net:master Jan 1, 2021
@mrbean-bremen
Copy link
Member

Ok, I merged the two performance PRs (and also a small fix I made).

@wieslawsoltes
Copy link
Contributor

Ok, I merged the two performance PRs (and also a small fix I made).

Great, will update my PR

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.

3 participants