TAChart: some fixes for source's extent
Original Reporter info from Mantis: Marcin Wiazowski
-
Reporter name:
Original Reporter info from Mantis: Marcin Wiazowski
- Reporter name:
Description:
I'm attaching a patch, that fixes some issues in TListChartSource / TCustomChartSource, and also solves one serious problem.
Let's start from the serious problem. Let's consider an example in Example.png. The series' extent will be in this case: X = [1 .. 3] and Y = [101 .. 103]. To make further description simpler, we'll take a look only at Y range.
Once extent is calculated in TCustomChartSource.BasicExtent(), it is internally cached. TListChartSource inherits from TListChartSource, and implements some nice optimization: when we change some point's Y value, in most cases we don't need to invalidate the whole cached extent - it's enough only to modify it, if needed.
For example: we want to change second point's Y value - this is performed by calling TListChartSource.SetYValue():
a) from Y=102 to Y = 100 -> new Y is below the current cached extent -> so update the cached extent to be [100 .. 103]
b) from Y=102 to Y = 101 -> new Y is within the current cached extent -> no action required
c) from Y=102 to Y = 103 -> new Y is within the current cached extent -> no action required
d) from Y=102 to Y = 104 -> new Y is above the current cached extent -> so update the cached extent to be [101 .. 104]
Another example: we want to change first point's Y value:
e) from Y=101 to Y = 100 -> new Y is below the current cached extent -> so update the cached extent to be [100 .. 103]
f) from Y=101 to Y = 102 -> we removed cached extent's bottom bound -> so cached extent needs to be fully recalculated
g) from Y=101 to Y = 103 -> we removed cached extent's bottom bound -> so cached extent needs to be fully recalculated
h) from Y=101 to Y = 104 -> we removed cached extent's bottom bound -> so cached extent needs to be fully recalculated
The problem is in TListChartSource.SetXValue() and SetYValue() - more precisely in their internal UpdateExtent() procedures. For cases f), g) and h), UpdateExtent() immediately recalculates the whole extent. This causes a problem when we want to shift every data point by some value:
for I := 0 to List.Count - 1 do
List.SetYValue(I, I + 567);
In our example, this will be:
- I=0: we removed cached extent's bottom bound, i.e. 101 -> cached extent is fully recalculated, by iterating all the existing data points
- I=1: we removed cached extent's bottom bound, i.e. 102 -> cached extent is fully recalculated, by iterating all the existing data points
- I=2: we removed cached extent's bottom bound, i.e. 103 -> cached extent is fully recalculated, by iterating all the existing data points
This way, something that was intended to make things faster (by updating the cached extent, instead of invalidating it), slows us horribly down: we iterate through 3 data points 3 times, i.e. we have List.Count ^ 2 data point accesses - so for List.Count like 1000, we will get 1000000 data accesses.
The attached ExtentSpeedTest application shows this:
- execution time with "Lucky case" option (adding -123456 to every point's Y): 0 ms
- execution time with "Unlucky case" option (adding +123456 to every point's Y): 7600 ms
The solution is quite simple: instead of recalculating the cached extent each time, we should just invalidate it. Then it will be calculated again when needed, which will be already after modifying all the data points.
After applying the patch:
- execution time with "Lucky case" option: 0 ms
- execution time with "Unlucky case" option: 0 ms
More detailed changelog:
Changes in tacustomsource.pas:
-
There is a protected function TCustomChartSource.CalcExtentXYList(), which was added about five weeks ago - I added lacking check for YCount > 1 there. By the way, I added also "UseYList" parameter (currently only "UseXList" parameter is used) to provide identical handling also for Y values - maybe someone will find this useful when inheriting from TCustomChartSource.
-
In TCustomChartSource.ExtentCumulative(), upper array range is now taken directly from "High(YList)", instead of "YCount - 2" - which is more safe, and also as it is already in other places.
-
In TCustomChartSource.GetErrorBarValues(), range check for XCount has been fixed (as a reference: 10 lines below there is same check for YCount).
Changes in tasources.pas:
-
No longer needed, local SourceClassString() function has been removed.
-
In TListChartSource.Delete(), lacking checks for XCount / YCount have been added.
-
In TListChartSource.SetXCount() / SetYCount(), lacking calls to InvalidateCaches() have ben added to invalidate the cached extent; all the other existing TCustomChartSource descendants call InvalidateCaches() properly.
-
In TListChartSource.SetXCount() / SetYCount() -> UpdateExtent(), lacking checks for XCount / YCount have been added, and the problem described above has been fixed.