views:

2824

answers:

4

I have a user control that contains a 2-column TableLayoutPanel and accepts commands to dynamically add rows to display details of an item selected in a separate control. So, the user will select a row in the other control (a DataGridView), and in the SelectedItemChanged event handler for the DataGridView I clear the detail control and then regenerate all the rows for the new selected item (which may have a totally different detail display from the previously selected item). This works great for a while. But if I keep moving from one selected item to another for quite a long time, the refreshes become VERY slow (3-5 seconds each). That makes it sound like I'm not disposing everything properly, but I can't figure out what I'm missing. Here's my code for clearing the TableLayoutPanel:

    private readonly List<Control> controls;

    public void Clear()
    {
        detailTable.Visible = false;
        detailTable.SuspendLayout();
        SuspendLayout();
        detailTable.RowStyles.Clear();
        detailTable.Controls.Clear();
        DisposeAndClearControls();
        detailTable.RowCount = 0;
        detailTable.ColumnCount = 2;
    }

    private void DisposeAndClearControls()
    {
        foreach (Control control in controls)
        {
            control.Dispose();
        }
        controls.Clear();
    }

And once I get finished loading up all the controls I want into the TableLayoutPanel for the next detail display here's what I call:

    public void Render()
    {
        detailTable.ResumeLayout(false);
        detailTable.PerformLayout();
        ResumeLayout(false);
        detailTable.Visible = true;
    }

I'm not using anything but labels (and a TextBox very rarely) inside the TableLayoutPanel, and I add the Labels and TextBoxes to the controls list (referenced in DisposeAndClearControls()) when I create them. I tried just iterating over detailTable.Controls and disposing them that way, but it seemed to miss half the controls (determined by stepping through it in the debugger). This way I know I get them all.

I'd be interested in any suggestions to improve drawing performance, but particularly what's causing the degradation over multiple selections.

A: 

Unfortunately, the only advice I can offer is to take care of the placement of your controls yourself. In my experience the .NET TableLayoutPanel, while very useful, is leaking SOMETHING and becomes unusably slow as it grows (and it doesn't take an unreasonable number of cells to get to this point, either). This behavior can be seen in the designer as well.

Adam Robinson
You seem to be right. If I'm reading the VSTS profiler data correctly, it's spending most of its time in the Clear and Visible = true operations.
Cory McCarty
A: 

I changed the containing form to just construct a new version of my user control on each selection change. It disposes the old one and constructs a new one. This seems to perform just fine. I'd originally gone with reusing just one for performance reasons anyway. Clearly that doesn't improve the performance. And the performance isn't a problem if I dispose the old one and create a new one.

Unfortunate that the TableLayoutPanel leaks like that, though.

Cory McCarty
+1  A: 

Hi! Just use a custom control that inherits from TableLayoutPanel and set the DoubleBuffered property on true, works great... especially when you dynamically add or remove rows.

public CustomLayout()
{
   this.DoubleBuffered = true;
   InitializeComponent();
}
Gorsh
+1  A: 

I had a similar issue with TableLayout. If I used TableLayout.Controls.Clear() method, the child controls never got disposed but when I simply dropped the TableLayout without clearing it, the leak stopped. In retrospect, it's funny I used the Clear method to prevent some kind of leak.

Apparently, Clear method does not explicitly dispose of the controls (which makes sense, because the fact that you removed them from the TableLayout does not mean you are done with them) and removing the child controls from the TableLayout prevents the cleanup routine to dispose of the children when the LayoutTable itself gets disposed (it simply does not know about them anymore).

My recommendation: Delete the detailTable.Controls.Clear(); line, remove the detailTable itself from the parent's Controls collection and dispose it, then create a brand new TableLayout for the next round. Also lose the DisposeAndClearControls method entirely since you won't need it. In my experience, it worked nicely.

This way, you won't have to recycle your entire user control anymore but only the TableLayout within.

Ishmaeel