How to Improve wxGrid performance Topic is solved

If you are using the main C++ distribution of wxWidgets, Feel free to ask any question related to wxWidgets development here. This means questions regarding to C++ and wxWidgets, not compile problems.
Post Reply
maxbld
Earned some good credits
Earned some good credits
Posts: 111
Joined: Wed Jan 30, 2013 10:49 pm

How to Improve wxGrid performance

Post by maxbld » Tue Mar 22, 2016 11:22 am

Dear all,

I've profiled my app, which is using wxGrid directly, without deriving wxGridTableBase nor wxGridCellAttrProvider. I'm querying a database and trying to show a table with 50+ columns and 2000+ rows, and it's very slow. The profiler shows that of 67854 samples, the 56,99% (38670) is spent in wxGrid::SetCellRenderer and 7,94% (5386) in wxGrid::SetCellEditor. I call those methods becouse I put buttons in some of the cells, based on an attribute I store in a wxArray. But what makes me think is that in the subsequent wxGrid::SetCellRenderer call stack what uses the most samples is wxGridTableBase::GetAttr (19525 samples) and wxGridTableBase::SetAttr (19034 samples) and in wxGrid::SetCellEditor wxGridCellAttrData::GetAttr takes 5373 samples over 5386. Now the code of such functions is quite simple:

Code: Select all

wxGridCellAttr *wxGridCellAttrData::GetAttr(int row, int col) const
{
    wxGridCellAttr *attr = NULL;

    int n = FindIndex(row, col); //HOT line
    if ( n != wxNOT_FOUND )
    {
        attr = m_attrs[(size_t)n].attr;
        attr->IncRef();
    }

    return attr;
}
void wxGridTableBase::SetAttr(wxGridCellAttr* attr, int row, int col)
{
    if ( m_attrProvider )
    {
        if ( attr )
            attr->SetKind(wxGridCellAttr::Cell);
        m_attrProvider->SetAttr(attr, row, col); //HOT line
    }
    else
    {
        // as we take ownership of the pointer and don't store it, we must
        // free it now
        wxSafeDecRef(attr);
    }
}
I've seen in this post viewtopic.php?t=3983 that one can derive wxGridCellAttrProvider, create the attributes with the renderes he wants at wxGridCellAttrProvider construction and clone them in an override of wxGridCellAttrProvider::GetAttr. Do you think that would work for my case? I mean would that improve performances by making not necessary to call wxGrid::SetCellRenderer and wxGrid::SetCellEditor? Or would the clone operation consume the same amount of samples?

What should I do? Should I just stop calling wxGrid::SetCellRenderer, create my own wxGridTableBase, set my own wxGridCellAttrProvider in it and use my property array to get the clone of the right attribute when GetAttr is called by the wxGrid?

And another thing... Here's wxGridCellAttr::Clone() code:

Code: Select all

wxGridCellAttr *wxGridCellAttr::Clone() const
{
    wxGridCellAttr *attr = new wxGridCellAttr(m_defGridAttr);

    if ( HasTextColour() )
        attr->SetTextColour(GetTextColour());
    if ( HasBackgroundColour() )
        attr->SetBackgroundColour(GetBackgroundColour());
    if ( HasFont() )
        attr->SetFont(GetFont());
    if ( HasAlignment() )
        attr->SetAlignment(m_hAlign, m_vAlign);

    attr->SetSize( m_sizeRows, m_sizeCols );

    if ( m_renderer )
    {
        attr->SetRenderer(m_renderer);
        m_renderer->IncRef();
    }
    if ( m_editor )
    {
        attr->SetEditor(m_editor);
        m_editor->IncRef();
    }

    if ( IsReadOnly() )
        attr->SetReadOnly();

    attr->SetOverflow( m_overflow == Overflow );
    attr->SetKind( m_attrkind );

    return attr;
}
Clone should be called only if the cell attribute doesn't exist yet, right? I guess GetAttr() is called again after the wxGrid is shown, so one should check if the wxGridCellAttr exists already, before calling the clone function, Am I wrong?

Best regards and thanks a lot for any answer,
Max.

User avatar
doublemax
Moderator
Moderator
Posts: 15074
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Re: How to Improve wxGrid performance

Post by doublemax » Tue Mar 22, 2016 12:58 pm

At this point you already know more about wxGrid's inner workings than i do. So i can't help any further with this. If nobody else has an idea, try the mailing list.
Use the source, Luke!

iwbnwif
Super wx Problem Solver
Super wx Problem Solver
Posts: 273
Joined: Tue Mar 19, 2013 8:52 pm

Re: How to Improve wxGrid performance

Post by iwbnwif » Tue Mar 22, 2016 3:50 pm

It has been some time ago, but I think we discussed something similar once before.

Did you see this thread ...

viewtopic.php?f=1&t=39113

I think to get good performance the answer was to avoid wxGridCellAttr and also to use wxGridUpdateLocker.
wxWidgets 3.1.2, MinGW64 8.1.0, g++ 8.1.0, Ubuntu 19.04, Windows 10, CodeLite + wxCrafter
Some people, when confronted with a GUI problem, think "I know, I'll use Eclipse RCP". Now they have two problems.

maxbld
Earned some good credits
Earned some good credits
Posts: 111
Joined: Wed Jan 30, 2013 10:49 pm

Re: How to Improve wxGrid performance

Post by maxbld » Wed Apr 27, 2016 4:37 pm

iwbnwif wrote:It has been some time ago, but I think we discussed something similar once before.

Did you see this thread ...

viewtopic.php?f=1&t=39113

I think to get good performance the answer was to avoid wxGridCellAttr and also to use wxGridUpdateLocker.
That was just part of the problem, the issue I was facing now was different. Actually when you use the standard implementation of the wxGridCellAttrProvider, you assign the attributes of the cells one by one while you build the wxGridStringTable. If you have many cells, the performances are killed.

I seen 2 solutions for this: loading the data in the moment the cell is painted, which can be done overriding wxGridStringTable::GetValue, and/or creating the attribute in that same moment, which can be done overriding wxGridCellAttrProvider::GetAttr. I think the real thing is going for first solution, but I started implementing the second, which seemed the easier. I've done the following:

Code: Select all

struct cellAttrList;
class GridCellButtonRenderer;
struct GridCellButtonRendererList;
struct GridCellButtonRendererRow;
WX_DECLARE_USER_EXPORTED_OBJARRAY(cellAttrList, ArrayCellAttr);
WX_DECLARE_USER_EXPORTED_OBJARRAY(GridCellButtonRendererList*, ArrayCellButtonRenderer);
WX_DECLARE_USER_EXPORTED_OBJARRAY(GridCellButtonRendererRow*, MatrixCellButtonRenderer);
struct GridCellButtonRendererRow{
    ArrayCellButtonRenderer* cellRendererArray;
    int row;
};
struct GridCellButtonRendererList{
    GridCellButtonRenderer* cellRenderer;
	int col;
};
#include <wx/arrimpl.cpp>
WX_DEFINE_USER_EXPORTED_OBJARRAY(ArrayCellButtonRenderer);
WX_DEFINE_USER_EXPORTED_OBJARRAY(MatrixCellButtonRenderer);
class VediDBGridCellAttrProvider: public wxGridCellAttrProvider {
	public:
    
	VediDBGridCellAttrProvider(Store* RSet);
    ~VediDBGridCellAttrProvider();

    wxGridCellAttr *GetAttr(int row, int col, wxGridCellAttr::wxAttrKind kind) const;

	private:
    int const TellIndex(int pos, ArrayCellAttr arrayAttr);
	Store* m_RSet;
	wxGridCellAttr *m_pNormAttr, *m_pDateAttr;
	ArrayCellAttr m_pButtonAttr, m_pComboAttr, m_pConfigComboAttr;
	MatrixCellButtonRenderer* m_BRenderers;
};
	VediDBGridCellAttrProvider::VediDBGridCellAttrProvider(Store* RSet) {
		int col;
		m_RSet = RSet;  
		m_pNormAttr = new wxGridCellAttr();
		m_RSet->InitCellAttrs(&m_pDateAttr, &m_pButtonAttr, &m_pComboAttr, &m_pConfigComboAttr);
		m_BRenderers = new MatrixCellButtonRenderer();
    }

    VediDBGridCellAttrProvider::~VediDBGridCellAttrProvider() {
		int i,j;
		m_pNormAttr->DecRef();
		m_pDateAttr->DecRef();
		for(i=0;i<m_pButtonAttr.GetCount();i++)
			m_pButtonAttr[i].cellAttr->DecRef();
		for(i=0;i<m_pComboAttr.GetCount();i++)
			m_pComboAttr[i].cellAttr->DecRef();
		for(i=0;i<m_pConfigComboAttr.GetCount();i++)
			m_pConfigComboAttr[i].cellAttr->DecRef();

		for(i=0;i<m_BRenderers->GetCount();i++){
			for(j=0;j<(*m_BRenderers)[i]->cellRendererArray->GetCount();j++){
				delete((*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer);
				delete((*(*m_BRenderers)[i]->cellRendererArray)[j]);
			}
			delete((*m_BRenderers)[i]->cellRendererArray);
			delete((*m_BRenderers)[i]);
		}
		delete(m_BRenderers);
    }

    wxGridCellAttr *VediDBGridCellAttrProvider::GetAttr(int row, int col, wxGridCellAttr::wxAttrKind  kind) const {
		if(m_RSet->GetAutosizing())
			return NULL;
		wxGridCellAttr *pAttr = NULL;
		VediDBSQLOStream ostream = m_RSet->GetParent()->GetTransaction()->getOStream();
		dba::Database::StoreType type = dba::Database::UNKNOWN;
		ArrayMap mapArray = m_RSet->GetData();
		int i = 0, j = 0, attrIndex;
		int index = m_RSet->GetRSetData()->TellIndexByPos(col);
		int nGridType = m_RSet->GetRSetData()->GetActiveType();
		if(index < 0)
			return pAttr;
		ostream.getColName(col, m_RSet, &type);

		if(type == dba::Database::DATE){
			pAttr = m_pDateAttr->Clone();
		}
		else if(mapArray[index].hasButton > 0){ 
			for(i=0; i<m_pButtonAttr.GetCount();i++){
				if(m_pButtonAttr[i].pos == col)
					break;
			}
			if(i<m_pButtonAttr.GetCount()){
				int rows = 0, cols = 0, i = 0;
				pAttr = m_pNormAttr->Clone();
				//pAttr = m_pButtonAttr[i].cellAttr->Clone();
				rows = m_BRenderers->GetCount();
				if(rows > 0){
					while(i<rows){
						if((*m_BRenderers)[i]->row == row){
							break;
						}
						i++;
					}
				}
				if(i == m_BRenderers->GetCount()){
					GridCellButtonRendererList *ButtonRenderer = new GridCellButtonRendererList();
					ArrayCellButtonRenderer *BRendererArray = new ArrayCellButtonRenderer();
					GridCellButtonRendererRow *ButtonRendererRow = new GridCellButtonRendererRow();
					ButtonRenderer->cellRenderer = new GridCellButtonRenderer(wxT(""), m_RSet);
					ButtonRenderer->col = col;
					BRendererArray->Add(ButtonRenderer);
					ButtonRendererRow->row = row;
					ButtonRendererRow->cellRendererArray = BRendererArray;
					m_BRenderers->Add(ButtonRendererRow);
					pAttr->SetRenderer((*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer);
					(*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer->IncRef();
				}
				else{
					cols = (*m_BRenderers)[i]->cellRendererArray->GetCount();
					if(cols > 0){
						GridCellButtonRenderer *cellRenderer = (*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer;
						int pos = (*(*m_BRenderers)[i]->cellRendererArray)[j]->col;
						while(j<(*m_BRenderers)[i]->cellRendererArray->GetCount()){
							if((*(*m_BRenderers)[i]->cellRendererArray)[j]->col == col){
								pAttr->SetRenderer((*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer);
								(*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer->IncRef();
								break;
							}
							j++;
						}
					}
					if(j == (*m_BRenderers)[i]->cellRendererArray->GetCount() || cols == 0){
						GridCellButtonRendererList *ButtonRenderer = new GridCellButtonRendererList();
						ButtonRenderer->cellRenderer = new GridCellButtonRenderer(wxT(""), m_RSet);
						ButtonRenderer->col = col;
						(*m_BRenderers)[i]->cellRendererArray->Add(ButtonRenderer);
						pAttr->SetRenderer((*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer);
						(*(*m_BRenderers)[i]->cellRendererArray)[j]->cellRenderer->IncRef();
					}
				}
			}
		}
		else if(mapArray[index].comboName != wxT("")){
			for(i=0; i<m_pComboAttr.GetCount();i++){
				if(m_pComboAttr[i].pos == col)
					break;
			}
			if(i<m_pComboAttr.GetCount())
				pAttr = m_pComboAttr[i].cellAttr->Clone();
		}
		else if(mapArray[index].configComboName != wxT("")){
			for(i=0; i<m_pConfigComboAttr.GetCount();i++){
				if(m_pConfigComboAttr[i].pos == col)
					break;
			}
			if(i<m_pConfigComboAttr.GetCount())
				pAttr = m_pConfigComboAttr[i].cellAttr->Clone();
		}
		if(nGridType == 1){
			m_RSet->SetOwnLayout(&pAttr, m_pNormAttr, mapArray[index].colName);	
		}
		return pAttr;
    }
	
	int const VediDBGridCellAttrProvider::TellIndex(int pos, ArrayCellAttr arrayAttr){
		int i;
		for(i=0; i<arrayAttr.GetCount();i++){
			if(arrayAttr[i].pos == pos)
				break;
		}
		if(i<arrayAttr.GetCount())
			return i;
		else
			return -1;
	}
Basically when the cells are painted I'm cloning the attributes that only need column level info. Instead I create each single cell attributes from the class GridCellButtonRenderer, for which I've to know the rectangle in which are painted one by one. These I store in a matrix where I write the grid rows numbers and at column level the grid col numbers and the attribute itself, so that at runtime the program knows whether the attribute for the current button cell was created already or it still has to be created from scratch for the first time.

This dramatically boosted the performances (more than ten times by what I've seen in the profiler), but I'd expect the best improvement from solution 1. If I'll do that, I'll post it here.

Post Reply