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

minor optimisations #1084

Merged
merged 3 commits into from
Oct 8, 2023
Merged

Conversation

AntonyCorbett
Copy link
Contributor

Set of small optimisations to reduce SOH allocations on hot paths. Provides ~20% reduction in time to process very large SVG data.

@AntonyCorbett
Copy link
Contributor Author

@wieslawsoltes I closed the single PRs and placed all changes in this one.

@wieslawsoltes
Copy link
Contributor

@AntonyCorbett Could you add benchmarks for those optimizations' https://github.com/svg-net/SVG/tree/master/Tests/Svg.Benchmark

for (var i = graphicsPath.PointCount - 1; i >= 0; --i)
if ((graphicsPath.PathTypes[i] & 0x7) == 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PathTypes allocates byte[] each time in this loop

@@ -17,7 +17,7 @@ public static string ToSvgString(this float value)

public static string ToSvgString(this PointF p)
{
return p.X.ToSvgString() + " " + p.Y.ToSvgString();
return $"{p.X.ToSvgString()} {p.Y.ToSvgString()}";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reduces string allocation in SOH, improving perf

@@ -111,7 +111,7 @@ public object Clone()

public override string ToString()
{
return string.Join(" ", this.Select(p => p.ToString()).ToArray());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary allocation to array

@AntonyCorbett
Copy link
Contributor Author

AntonyCorbett commented Aug 30, 2023

@wieslawsoltes

@AntonyCorbett Could you add benchmarks for those optimizations' https://github.com/svg-net/SVG/tree/master/Tests/Svg.Benchmark

My simple test involves the use of a 30MB SVG file (so not sure that you would want me to include it). The benchmark code is as follows:

[Benchmark]
public void SvgDraw_BigRiver()
{
    using var stream = Open("__BigRiver.svg");
    var doc = SvgDocument.Open<SvgDocument>(stream);
    var bmp = doc.Draw();
    using MemoryStream ms = new MemoryStream();
    bmp.Save(ms, ImageFormat.Jpeg);
}

Results for non-optimised code:

test1

Results for optimised code:

test2

@wieslawsoltes
Copy link
Contributor

@wieslawsoltes

@AntonyCorbett Could you add benchmarks for those optimizations' https://github.com/svg-net/SVG/tree/master/Tests/Svg.Benchmark

My simple test involves the use of a 30MB SVG file (so not sure that you would want me to include it). The benchmark code is as follows:

[Benchmark]
public void SvgDraw_BigRiver()
{
    using var stream = Open("__BigRiver.svg");
    var doc = SvgDocument.Open<SvgDocument>(stream);
    var bmp = doc.Draw();
    using MemoryStream ms = new MemoryStream();
    bmp.Save(ms, ImageFormat.Jpeg);
}

Results for non-optimised code:

test1

Results for optimised code:

test2

Looking great 👍

Btw what I meant for benchmark is to be more granular and cover just one optimization, you can see the existing benchmarks are done that way. It helps preventing regression and easier finding what those caused.

@@ -39,19 +39,19 @@ public override Brush GetBrush(SvgVisualElement renderingElement, ISvgRenderer r
chain.Add(curr);
curr = SvgDeferredPaintServer.TryGet<SvgPatternServer>(curr.InheritGradient, renderingElement);
} while (curr != null);

var firstChildren = chain.Where(p => p.Children.Count > 0).FirstOrDefault();

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove spaces.

@mrbean-bremen mrbean-bremen merged commit 4fd6cd8 into svg-net:master Oct 8, 2023
7 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 8, 2023
…erators Nuget README.md Samples Source Svg.Custom Tests doc docfx.json index.md license.txt minor optimisations BuildProcessTemplates CONTRIBUTING.md Generators Nuget README.md Samples Source Svg.Custom Tests doc docfx.json index.md license.txt avoid materialisation of enumerable.
github-actions bot pushed a commit to H1Gdev/SVG that referenced this pull request Oct 8, 2023
….md Generators Nuget README.md Samples Source Svg.Custom Tests doc docfx.json index.md license.txt minor optimisations BuildProcessTemplates CONTRIBUTING.md Generators Nuget README.md Samples Source Svg.Custom Tests doc docfx.json index.md license.txt avoid materialisation of enumerable.
@paulushub paulushub mentioned this pull request Jan 19, 2024
6 tasks
paulushub added a commit that referenced this pull request Jan 19, 2024
* Fixes empty path issue

- Fixes issue #1095
- Broken with: Minor optimisations (#1084) commit

* Update ReleaseNotes.md
github-actions bot pushed a commit that referenced this pull request Jan 19, 2024
…amples Source Svg.Custom Tests doc docfx.json index.md license.txt Fixes empty path issue - Fixes issue #1095
 - Broken with: Minor optimisations (#1084) commit
 
 CONTRIBUTING.md Generators README.md Samples Source Svg.Custom Tests doc docfx.json index.md license.txt Update ReleaseNotes.md
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.

5 participants