ColorGrid unacceptable performance

[CChung]

Hello,

I would like to add that like rvs we are also in need of a very fast colorgrid. (BTW, great work, rvs!) Following this discussion it would seem to me that an unmanaged version should be capable of dramatic increases in speed. This would be very much appreciated as we are in need of a colorgrid that is at least ten times as fast as in the current version (not the debug version which I haven't tried yet) meaning 1000x1000 in around a second.

And while you are at it, could you add a fast surface series as well? Especially since I cannot seem to get OpenGL canvas to work. (I added a new topic for this)

Chuck

[zema]

Hi, Christopher

Have you looked at my bug reports in the last several posts in this topic? Those are very annoying ones, really.

Regards,
Alexander

[Christopher]

Hello rvs,

Have you looked at my bug reports in the last several posts in this topic? Those are very annoying ones, really.

Yes, thank you, they are fixed.

[zema]

Hi,

When are you planning to release the new build?

Regards,
Alexander

[Narcís]

Hi Alexender,

We expect to have the next debug build ready by the end of this week or next week's beginning.

[zema]

Hi,

Thanks a lot for the great work. I'm now almost satisfied with how ColorGrid. But still several questions are open:

• Is the this.bbitmap = (Bitmap) this.bitmap.Clone(); line in the ColorGrid.DrawCellUsingBitmap really necessary? You could use ColorGrid.bitmap instead of ColorGrid.bbitmap and not clone the backbuffer bitmap each time it's painted. GC is trying hard to avoid using all the CPU time, but you're making its life harder.

• One more memory usage issue: TeeChart allocates and releases about 2-3 Mb on each paint for 500x500 grid. This is not critical, but slows down the program if it repaints color grid frequently. This issue is unrelated to the previous one with cloning the bitmap, as I'm using my FastColorGrid class which doesn't clone bitmap each time it's painted.

• ColorGrid overlaps bottom border of the chart:
  
  This is also not a critical bug, but somewhat annoying.

Regards,
Alexander

[zema]

Hi,

I've got several more things to say regarding performance.

I've profiled the test application that uses my FastColorGrid class and was a kind of surprised with the results (these are the most useful extracts from the full profiling report):

Code: Select all

100.00%   Draw  -  27100 ms  -  16 calls  -  Steema.TeeChart.TChart.Draw(Graphics)
  99.69%   InternalDraw  -  27015 ms  -  16 calls  -  Steema.TeeChart.Chart.InternalDraw(Graphics)
    99.67%   InternalDraw  -  27011 ms  -  16 calls  -  Steema.TeeChart.Chart.InternalDraw(Graphics, Boolean)
      98.38%   DoBeforeDrawChart  -  26661 ms  -  16 calls  -  Steema.TeeChart.Styles.Custom3DGrid.DoBeforeDrawChart()
        98.20%   FillGridIndex  -  26614 ms  -  16 calls  -  Steema.TeeChart.Styles.Custom3DGrid.FillGridIndex(Int32)
          98.15%   FillRegularGrid  -  26598 ms  -  16 calls  -  Steema.TeeChart.Styles.Custom3DGrid.FillRegularGrid(Int32, Int32 &, Int32 &, Double, Double)
            54.41%   set_Item  -  14745 ms  -  4000000 calls  -  Steema.TeeChart.Styles.Custom3DGrid.set_Item(Int32, Int32, Int32)
              47.00%   InternalSetGridIndex  -  12737 ms  -  4000000 calls  -  Steema.TeeChart.Styles.Custom3DGrid.InternalSetGridIndex(Int32, Int32, Int32)
                16.28%   Add  -  4412 ms  -  16000000 calls  -  System.Collections.Generic.List<T>.Add(T)
                3.79%   set_Item  -  1026 ms  -  4008000 calls  -  System.Collections.Generic.List<T>.set_Item(Int32, T)
              0.00%   List<T>..cctor  -  0 ms  -  1 call  -  System.Collections.Generic.List<T>..cctor()
            7.94%   ToInt32  -  2151 ms  -  8000064 calls  -  System.Convert.ToInt32(Double)
            6.93%   get_Item  -  1877 ms  -  8000000 calls  -  Steema.TeeChart.Styles.ValueList.get_Item(Int32)
            4.47%   get_Count  -  1212 ms  -  4000016 calls  -  Steema.TeeChart.Styles.Series.get_Count()
            0.06%   ClearGridIndex  -  17 ms  -  16 calls  -  Steema.TeeChart.Styles.Custom3DGrid.ClearGridIndex()
            0.00%   get_Maximum  -  0 ms  -  32 calls  -  Steema.TeeChart.Styles.ValueList.get_Maximum()
          0.05%   get_Minimum  -  14 ms  -  32 calls  -  Steema.TeeChart.Styles.ValueList.get_Minimum()
        ...
      0.95%   DrawSeries  -  256 ms  -  16 calls  -  Steema.TeeChart.Styles.Series.DrawSeries()
        0.87%   Draw  -  236 ms  -  16 calls  -  WindowsApplication1.FastColorGrid.Draw()
          0.86%   DrawCellUsingBitmap  -  232 ms  -  16 calls  -  WindowsApplication1.FastColorGrid.DrawCellUsingBitmap(Rectangle &, Rectangle &)
            0.49%   FillBitmap  -  133 ms  -  16 calls  -  WindowsApplication1.FastColorGrid.FillBitmap(Rectangle &, Bitmap)
            0.33%   DrawBitmap  -  88 ms  -  16 calls  -  WindowsApplication1.FastColorGrid.DrawBitmap(Bitmap, Rectangle &)
              0.31%   Draw  -  84 ms  -  16 calls  -  Steema.TeeChart.Drawing.Graphics3DGdiPlus.Draw(Rectangle, Image, Boolean)
              0.01%   PrepareDrawImage  -  2 ms  -  16 calls  -  Steema.TeeChart.Drawing.Graphics3DGdiPlus.PrepareDrawImage()
              0.00%   get_Graphics3D  -  0 ms  -  16 calls  -  Steema.TeeChart.Chart.get_Graphics3D()

(I've used JetBrains dotTrace for profiling. They provide full functional 30 days trial.)

You can see from the report above that 98.15% of time is spent in the FillRegularGrid. I found a few places in your code which could run much faster:

• Utility.Round should better use Math.Round(double) instead of Convert.ToInt32(double), as it is approx. 3 times faster

• Custom3DGrid.ClearGridIndex should better Clear gridIndex instead of assigning them to null. This should also help to solve memory reallocation problems.

• You could replace Custom3DGrid.CellsRow..ctor() with something like this:

  Code: Select all

  CellsRow() : base(0x7d0) {}

  . This will remove unnecessary reallocations of the underlying data buffer.

• At last, it's a good idea to assign ReuseGridIndex to false when the XValues or ZValues collections are changed only. And set it to true when the grid index is filled to avoid useless index recreations.

Just imagine, that with these optimizations the speed of drawing will increase about 50 times (in the case FastColorGrid is used)! And the total performance increase comparing to the last release version would be about several hundred or thousand times! (Guys, you owe me a beer )

Best regards,
Alexander

[Christopher]

Hello Alexander,

Is the this.bbitmap = (Bitmap) this.bitmap.Clone(); line in the ColorGrid.DrawCellUsingBitmap really necessary?

It was when the code was written but has since become redundant and has now been removed.

ColorGrid overlaps bottom border of the chart:

OK. I've been able to stop cells overrdrawing the Back Wall pen by making a small change to CalcDestRectangle().

(I've used JetBrains dotTrace for profiling. They provide full functional 30 days trial.)

Thank you. We use AQTime, although I wish I had more time to use it!

Utility.Round should better use Math.Round(double) instead of Convert.ToInt32(double), as it is approx. 3 times faster

Unfortunately Math.Round returns a double (or decimal etc) whereas Utils.Round returns an integer, which is what is needed for drawing pixels.

Custom3DGrid.ClearGridIndex should better Clear gridIndex instead of assigning them to null. This should also help to solve memory reallocation problems.

In a managed .NET environment there aren't any memory reallocation problems
Unfortunately, using gridIndex.Clear() in ClearGridIndex() causes an error in the private method InternalSetGridIndex() and so cannot be implemented.

You could replace Custom3DGrid.CellsRow..ctor() with something like this:

In the .NET 1.1 version of TeeChart this constructor looks like this:

Code: Select all

public CellsRow() : base(MaxAllowedCells) {}

I've changed the .NET 2.0 code around so that it looks similar.

At last, it's a good idea to assign ReuseGridIndex to false when the XValues or ZValues collections are changed only. And set it to true when the grid index is filled to avoid useless index recreations.

I'm not sure if it would take just as long to check for changes in all the valuelists as it does to call FillGridIndex() on every paint.

(Guys, you owe me a beer Very Happy)

Well, have one (or two) on Steema . Seriously, many thanks for your interest in TeeChart and for all of your useful suggestions.

[zema]

Hi, Christopher

Unfortunately Math.Round returns a double (or decimal etc) whereas Utils.Round returns an integer, which is what is needed for drawing pixels.

Are you kiddin'? Have you forgot casts? You can use (double)Math.Round(x) instead of Convert.ToInt32(x) and it'll be appr. 3 times faster (as it's implemented as the native function.

In a managed .NET environment there aren't any memory reallocation problems

They are. But they are known as performance problems.

Unfortunately, using gridIndex.Clear() in ClearGridIndex() causes an error in the private method InternalSetGridIndex() and so cannot be implemented.

You make me wonder. Could you add a gridIndex[n].Count > 0 check to gridIndex[n] != null? I'm sure this should solve the problem.

I'm not sure if it would take just as long to check for changes in all the valuelists as it does to call FillGridIndex() on every paint.

You're already using ValueList.statsOk flag to determine if the stats are not up-to-date. Why not replace it with the version field and increment it on every change? (FYI, CLR collections use this way to allow tracking changes) This wouldn't slow down anything and would allow rebuilding grid index only when this is really needed.


Regards,
Alexander

[zema]

oops, I'm sorry, I mean (int)Math.Round(x), not (double)Math.Round(x). hope, you've understood this.

[zema]

and one more thing: Math.Round is faster than Convert.ToInt32, 'cause it's implemented as a native method, not as CLI. but it's functionality is the same.

[Christopher]

Hello Alexander,

Are you kiddin'? Have you forgot casts? You can use (double)Math.Round(x) instead of Convert.ToInt32(x) and it'll be appr. 3 times faster (as it's implemented as the native function.

I see. I haven't had time to test whether (int)Math.Round(x) is faster than Convert.ToInt32(x) but I've taken your word for it and have made the change.

You make me wonder. Could you add a gridIndex[n].Count > 0 check to gridIndex[n] != null? I'm sure this should solve the problem.

Using gridIndex.Clear() in ClearGridIndex() resets gridIndex.Count to zero, meaning that any calls to gridIndex[x] in InternalSetGridIndex() will fail.
I guess the problem here is that gridIndex was orginally created using nested ArrayLists/Lists<T> rather than a two-dimensional array, which would have solved this problem as Array.Clear() could then have been used which doesn't reset the count.
There might be some mileage in making gridIndex a two-dimensional array and seeing what performance differences exist. I'll add it as a wish.

You're already using ValueList.statsOk flag to determine if the stats are not up-to-date. Why not replace it with the version field and increment it on every change? (FYI, CLR collections use this way to allow tracking changes) This wouldn't slow down anything and would allow rebuilding grid index only when this is really needed.

Yes, I like that idea. Using reflection I can see the _version : Int32 field .. it's a shame it isn't public as it could be very useful in inherited classes!

[zema]

Hi, Christopher

I see. I haven't had time to test whether (int)Math.Round(x) is faster than Convert.ToInt32(x) but I've taken your word for it and have made the change.

You can be sure it is faster. I've made tests.

There might be some mileage in making gridIndex a two-dimensional array and seeing what performance differences exist. I'll add it as a wish.

Unfortunately, two-dimensional arrays are slower than arrays of arrays in CLR. And my idea of Clearing the gridIndex[x] was bad too. The good one is to fill gridIndex[x] with -1's. This will not lead to errors in InternalSetGridIndex and will not require reallocations. And furthermore, setting List<T> items is faster then Adding them. So, the solution is to change ClearGridIndex to something like this:

Code: Select all

private void ClearGridIndex()
{
      for (int i = 0; i < gridIndex.Count; ++i)
      {
            CellsRow row = gridIndex[i];
            if(row != null)
            {
                    for(int j = 0; j < row.Count; ++j)
                        row[j] = -1;
            }
      }
}

Yes, I like that idea. Using reflection I can see the _version : Int32 field .. it's a shame it isn't public as it could be very useful in inherited classes!

As far as ValueList is your own class and the version field will belong to it, you don't need to use reflection, you just can make a public getter for this variable

Best regards,
Alexander

[Christopher]

Hello Alexander,

Unfortunately, two-dimensional arrays are slower than arrays of arrays in CLR. And my idea of Clearing the gridIndex[x] was bad too. The good one is to fill gridIndex[x] with -1's. This will not lead to errors in InternalSetGridIndex and will not require reallocations. And furthermore, setting List<T> items is faster then Adding them. So, the solution is to change ClearGridIndex to something like this:

Thank you for the idea, it is a good one and has been implemented in the TeeChart code. I'm sure that not having to reallocate memory by creating a new instance of CellsRow on every paint is a speed improvement, although I haven't tested it. I'm not sure, however, that the line:

Code: Select all

gridIndex[x]=new CellsRow();

calls the Add method if the List<T> class; Reflection indicates that it assigns the values to the underlying _items : T[] directly.

As far as ValueList is your own class and the version field will belong to it, you don't need to use reflection, you just can make a public getter for this variable

So I understand. What I was saying was that having the version field as a public property in the ArrayList class would be very useful when implementing classes which inherited from it; if this field was public and ValueList inherited from ArrayList then I wouldn't have to implement it myself!!

[zema]

Hello, Christopher

I'm not sure, however, that the line:

Code:
gridIndex[x]=new CellsRow();

calls the Add method if the List<T> class; Reflection indicates that it assigns the values to the underlying _items : T[] directly.

What I'm talking about, is the following code in the InternalSetGridIndex:

Code: Select all

gridIndex[x] = new CellsRow();
for (int i = 0; i < 0x7d0; i++)
{
       gridIndex[x].Add(-1);
}

which for sure calls List<T>.Add(...). And using the Add method is less efficient than assigning items through the indexer.

Could you, please, make a debug build when you are done with implementing all these fixes and performing some testing? It would be nice for me to have an optimized version, as we are about to release our application one of the parts of which depends on the ColorGrid performance.

Best regards,
Alexander