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

Reduce gc allocations on render loop #792

Merged
merged 9 commits into from
Dec 3, 2022

Conversation

pkindruk
Copy link
Contributor

I have been investigating performance and memory issues on my app recently and mentioned that LiveCharts allocate a lot of single use data on every update. This introduces GC pressure which can be avoided.

I created a test workload to highlight those issues. It has 14x4k LineSeries, 4k FinancialSeries, 150k FinancialSeries and 2 ScatterSeries of 8K points on both. Responsiveness in that setup is really bad. It moves, but has huge lags. Though this story is for another time.

This PR focuses on reducing GC allocation on render loop.

Let's take a look at allocations during render (GC Heap Alloc Ignore Free Stacks). I used PerfView Collect command here, app was already started and data loaded for 10-20 seconds. This highlights what types are being constantly allocated.

gc-pressure1 0

gc-pressure1 1

As we can see main allocation sources are:

  • BezierData
  • RectangleHoverArea
  • HashSet<ChartPoint> copy constructor

There is also a tricky case with float is null. For some reason JITter doesn't optimize boxing when using launching app using PerfView Run command. This doesn't reproduce on normal start (or at least I can't find evidence). However I need that fix in MotionProperty<T> keep recorded performance data clear. JITter should treat static readonly bool fields as constants at IL->Asm compilation and simplify comparison.

gc-pressure1 2

@pkindruk
Copy link
Contributor Author

Change list:

  1. BezierData turned to struct. Usage is limited to generator functions and stack of LineSeries.Invalidate and PolarLineSeries.Invalidate functions. Can probably go further with readonly structs.
  2. ChartPointContext.HoverArea now initialized lazy.
  3. Replaced copy logic for Series.everFetched. Now it uses ChartPoint.RemoveOnCompleted and counts number points that should be removed.

As for boxing I used approach mentioned in this StackOverflow question.
JITter should treat static readonly bool fields as constants at IL->Asm compilation and simplify comparison.
However issues is probably still present for Nullable<T>. But right now there is no evidence, so I skipped it. Currently there are only 2 usages for MotionProperty<Nullable<T>>.

With this fix allocations during render look like this:
gc-pressure2

@beto-rodriguez
Copy link
Owner

beto-rodriguez commented Dec 3, 2022

Thanks for profiling the library, your PR is for sure reducing the memory consumption, this is something that will improve for sure the library.

It is smart what you did, I was building a new HashSet containing the points in the UI, it seems that the points are allocated twice with my approach, but your approach is using the current HashSet and just removing the points that were not used, in general your alternative seems smarter.

About the bad performance you faced, I am also working in a high-performance package, it improves
drastically how things are render in the UI, you can get a clue here.

Thanks this is definitely better.

@beto-rodriguez
Copy link
Owner

Those comments summarize the changes I made, thanks again for this PR!

@beto-rodriguez beto-rodriguez changed the base branch from master to dev December 3, 2022 15:00
@beto-rodriguez beto-rodriguez merged commit 166e272 into beto-rodriguez:dev Dec 3, 2022
beto-rodriguez added a commit that referenced this pull request Dec 19, 2022
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.

2 participants