search

CollaboraOnline / online

Collabora Online is a collaborative online office suite based on LibreOffice technology. This is also the source for the Collabora Office apps for iOS and Android.
https://collaboraonline.com
Other
1.85k stars 702 forks source link

calc perf: GetEmptyLinesInBlock #7332

Open mmeeks opened 1 year ago

mmeeks commented 1 year ago

From this week's calc flamegraph:

https://user-images.githubusercontent.com/833656/271274520-6345ee72-2a06-402e-a12c-72bd8d49df8d.svg

Shows a chunk of rendering time is calling:

image

GetEmptyLinesInBlock - we should examine that in some detail to see why :-)

mmeeks commented 1 year ago

sc/source/ui/view/output2.cxx:void ScOutputData::DrawEdit(bool bPixelToLogic) ... //! store nLastContentCol as member! SCCOL nLastContentCol = mpDoc->MaxCol(); if ( nX2 < mpDoc->MaxCol() ) nLastContentCol = sal::static_int_cast( nLastContentCol - mpDoc->GetEmptyLinesInBlock( nX2+1, nY1, nTab, mpDoc->MaxCol(), nY2, nTab, DIR_RIGHT ) );

A useful comment there to store the nLastContentCol I guess ...

mmeeks commented 1 year ago

Essentially duplicate logic in ScOutputData::LayoutStrings - doing the same thing; I guess MaxCol() increases bites us hard here too: in all 40% of the time in DrawContent and 25% of the paintPartTile time is scanning columns to the right to look for content ...

mmeeks commented 1 year ago

source/core/data/column2.cxx:SCSIZE ScColumn::GetEmptyLinesInBlock( SCROW nStartRow, SCROW nEndRow, ScDirection eDir ) const ... sc/source/core/data/column2.cxx- std::pair<sc::CellStoreType::const_iterator,size_t> aPos = maCells.position(nStartRow);

Seems to be using only the maCells - data storage; and we -should- have a document-wide maximum for this already stored in: ScTable's mnEndCol (I expect):

sc/source/core/data/table1.cxx-bool ScTable::GetCellArea( SCCOL& rEndCol, SCROW& rEndRow )

And I expect that mnEndCol <<< MaxCol() which should let us significantly speed this up.

@eszkadev I'm interested in how mnEndCol is maintained though - I'd expect to see more invalidation of mbCellAreaDirty as data is written into the document, but perhaps I miss something.

eszkadev commented 1 year ago

data area is invalidated here: https://opengrok.libreoffice.org/xref/core/sc/source/ui/unoobj/docuno.cxx?r=2f85eb7a#3298

mmeeks commented 1 month ago

@sopyb this might be an interesting one to look at too - quite some code pointers; I expect that its necessary to have a sheet with a lot of columns that get created perhaps in error to play with, perhaps @grandinj has an idea of a good test sheet to reproduce this before digging into it deeper ? =)