Performance + Crash fix (#1892) (#1895) #1947
Conversation
Replace all "new FSize()" instantiations with FSize.getInstance() / FSize.recycleInstance() pairs. Smarter FSize array usage inside Legend.
Replace all "new PointD()" instantiations with PointD.getInstance() / PointD.recycleInstance() pairs. Helper methods overloaded to include void return / output PointD variable signatures. Old methods remain as convenience, with notations that they return recyclable instances.
In addition to creating a resized label sizes array, copy old FSize instances to the new array to cut down on the need to get instances from pool. Recycle to pool if the labels shrank.
In order to have a poolable alternative to PointF, this changeset introduces MPPointF. Convenience methods exist that accept output MPPointF variables for extra savings on pool access. Methods that return recyclable MPPointF instances are documented.
Creating field level buffers for frequently instantiated float arrays. Cuts instantiations of many small objects.
Created buffers for short lived Matrix and Path objects, resetting them instead of instantiating new ones. Also created some matrix transform methods that accept output Matrix objects to facilitate caching.
LineChartRenderer now caches drawn circles as bitmaps. ILineDataSet now has a method that returns the number of available colors in the set.
foreach(:) arrays on Android carry the cost that each iteration creates an implicit instantiation. for(;;) arrays are uglier but guarantee that no unwanted allocations occur while iterating.
ValueFormatter objects now rely on a FormattedStringCache to return already-formatted Strings. We might want to create primitive-enabled versions since all our values to format are float or double primitives, and the keys are also all primitives. This will eliminate auto-boxing penalties.
Buffering temporary Rect and RectF instances. Created methods to modify RectF instances as performance alternatives to methods that create disposable RectF instances.
getXBounds creates a disposable XBounds instance on every call. Cache it and update the values with a set method. This means XBounds values are now mutable.
By creating versions of the FormattedStringCache that don't require auto boxing, allocations are cut tremendously in this commit.
paint.getFontMetrics() has a hidden allocation. paint.getFontMetrics(fontMetrics) is used internally inside of Paint, so caching and holding onto a metrics buffer helps reduce allocations.
Many short-lived ArrayLists were created in various methods, which could easily have been extracted to field level variable. This has been done, and the arrays cleared before use.
Created unit tests for the FormattedStringCache classes. Also, took this opportunity to create a FormattedStringCache.Generic<K,V> static class to take over what was previously simply FormattedStringCache. Verified that all unit tests still pass.
Found an array that was instantiated frequently without need. Placed bounds on its instantiation.
Replenish fewer objects in utils pools when empty, in case the pool size grows large.
Create object pools for the Zoom, move, and animation jobs. Their constructors remain public, to make this easier to roll back from if needed.
Noticed a couple of stray un-recycled MFPoint and FSize instances. These accounted for a bit of the remaining generated garbage.
Make sure we copy List<> into [] if the size is identical to the length, instead of instantiating a new [].
Zooming in on the circle bitmaps, they were slightly small. Allocate a little more space in the circle's bitmap to correct this.
With large data sets, the Path object created was sufficiently large as to cause an OutOfMemory error. This is resolved by only pathing a limited number of points on the chart at a time, then clearing the path and resuming. Stress testing with 1500 entries.
This reverts commit d5df3ad.
With large data sets, the Path object created was sufficiently large as to cause an OutOfMemory error. This is resolved by only pathing a limited number of points on the chart at a time, then clearing the path and resuming. Stress testing with 1500 entries.
This changeset merges all performance updates related to PhilJay#1892, as well as the bug fix for PhilJay#1895 for master.
|
Is this actually tested and proved to be worth it? |
|
Also all of that overhead just for the sake of pooling long-running objects like a formatter in the example, or an ZoomJob - that's just nonsense. @PhilJay ? |
|
The two allocation charts show the app running at its default 500 allocation example. Format string caching is a significant part of the memory allocation difference in these readings. The master branch incurred one GC run before the end of the 500 entries, and was near to incurring a second. At 5000 entries, master incurred garbage collection about 15 times. The merged feature branch allocates a fraction of the memory. Even at 5000 entries, it was not remotely close to incurring garbage collection. The results from gfx info indicate that after all the changes the worst case screen rendering times improve between 25%-40%. The rendering time per frame is much more consistent with these updates. GC hiccups account for some of this difference. The rest is accounted for by caching the the drawn circles inside the LineChartRenderer, which ends up far less expensive computationally than re-drawing all the circles during each onDraw() call. As it stands, the end user neglecting to recycle an FSize or MPPointF doesn't result in issues beyond the original issue of wasted memory. |
|
How is caching the string formatter giving you any benefit? It does not make any sense. These are set once - and reused as a variable. There's no reason to cache them. Well if the user neglecting to recycle an FSize or a point is not an issue - then it's logical to consider it not being an issue if not taken from the pool at all. It seems like there's some good work here on the object pools, but in some cases it's just an overkill... |
|
Caching the formatted string results prevents the application from calling // avoid memory allocations here (for performance) Unfortunately mFormat.format() is very allocation-hungry all on its own, As for failing to recycle the points not being an issue in terms of correct *Tony Patino *| Development Lead On Sat, Aug 6, 2016 at 11:14 AM, Daniel Cohen Gindi <
|
This pull request contains --
Note-- This change set will require approving a pull request in the MPAndroidChart-Realm project, to account for a new abstract method to retrieve the number of circle colors. PhilJay/MPAndroidChart-Realm#1