ScFormulaCell size foo ...

classic Classic list List threaded Threaded
3 messages Options
Michael Meeks-5 Michael Meeks-5
Reply | Threaded
Open this post in threaded view
|

ScFormulaCell size foo ...

Hi there,

        So - poking at some Massif data (kindly generated by Matus) for a large
Calc sheet - with ~300k rows containing ~11 (repeated) formula in each
row - I was interested by the biggest (long term) allocation:

83.61% (1,732,208,505B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
->20.54% (425,582,672B) 0x16CEE0F3: oox::xls::(anonymous namespace)::applyCellFormulas(ScDocumentImport&, oox::xls::(anonymous namespace)::CachedTokenArray&, SvNumberFormatter&, com::sun::star::uno::Sequence<com::sun::star::sheet::ExternalLinkInfo> const&, std::vector<oox::xls::FormulaBuffer::TokenAddressItem, std::allocator<oox::xls::FormulaBuffer::TokenAddressItem> > const&) (formulabuffer.cxx:210)
| ->20.54% (425,582,672B) 0x16CEE921: oox::xls::FormulaBuffer::finalizeImport() (formulabuffer.cxx:307)
|   ->20.54% (425,582,672B) 0x16D59131: oox::xls::WorkbookHelper::finalizeWorkbookImport() (workbookhelper.cxx:760)
|     ->20.54% (425,582,672B) 0x16D56250: oox::xls::WorkbookFragment::finalizeImport() (workbookfragment.cxx:494)

        Which is essentially 425Mb of:

                new ScFormulaCell(...)

        I had a look at the size of ScFormulaCell which is 152 bytes
which breaks down thus (Linux / 64bit):

        ScFormulaCell 152 bytes
                SvtListener 56 bytes
                ScFormulaResult 16 bytes
                ScAddress  8 bytes
                <fields> 72 bytes.

        The SvtListener is 1/3rd of the size of that. Interestingly we have
~2.8m of these ScFormulaCells in my sample.

        I knocked up the attached patch - assuming that we don't really use
that Listener nearly as much with the new FormulaGroup listener magic -
but ... it turns out that this increases memory usage:

* before
000000000126d000 1758056K 1758056K 1758056K 1758056K      0K rw-p [heap]
0000000001254000 1757728K 1757728K 1757728K 1757728K      0K rw-p [heap]

* after
0000000001478000 1783812K 1783812K 1783812K 1783812K      0K rw-p [heap]
000000000167a000 1782268K 1782268K 1782268K 1782268K      0K rw-p [heap]

        Which is interesting. My hope is that as/when we use the formula-group
listener more effectively, that the ScFormulaCell listener will not
actually be used and this patch will start to have a useful effect :-) I
suspect that we are pushing an entry into it currently for every
single-cell reference.

        My hope is that having a single-cell group listener construct for this
would mean that my tweak might actually work & save a ton of that
duplication / memory allocation. But that's for the future I guess.

        Other things that look a bit wasteful are:

    ScFormulaCell*  pPrevious;
    ScFormulaCell*  pNext;
    ScFormulaCell*  pPreviousTrack;
    ScFormulaCell*  pNextTrack;

        At least one pair of these (IIRC the 'Track') are needed only
transiently during calculation as an append-only list and could be
reasonably easily replaced by a bool bit-field and a std::vector on the
ScDocument itself - which would save 48Mb or so (on 64bit).

        Anyhow - just some thoughts =) I'm dropping this for now myself.

        ATB,

                Michael.

--
 [hidden email]  <><, Pseudo Engineer, itinerant idiot

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

0001-svl-Make-SvtListener-more-efficient-when-it-has-no-l.patch (6K) Download Attachment
Eike Rathke-2 Eike Rathke-2
Reply | Threaded
Open this post in threaded view
|

Re: ScFormulaCell size foo ...

Hi Michael,

On Wednesday, 2014-12-31 10:48:48 +0000, Michael Meeks wrote:

> I knocked up the attached patch - assuming that we don't really use
> that Listener nearly as much with the new FormulaGroup listener magic -
> but ... it turns out that this increases memory usage:

Not to much surprise ;-)

IF broadcasters are used frequently, the separately new'ed instances add
overhead for the heap allocation as then for each allocation also the
allocation structure is allocated. The sc::FormulaGroupAreaListener
singles out listening to a range/area that is identical in a group of
formulas, not to individual cells, which may differ for each formula in
a group.

If the ScFormulaCell instances were still allocated from the shared
mempool we'd also have slightly less allocation overhead with the
current code, but that is unrelated to the observation made here. Since
introduction of #ifdef USE_MEMPOOL in sc/inc/formulacell.hxx and
sc/source/core/data/formulacell.cxx the shared mempool is disabled.
Why anyway?


> I suspect that we are pushing an entry into it currently for every
> single-cell reference.

Yes.

> My hope is that having a single-cell group listener construct for this
> would mean that my tweak might actually work & save a ton of that
> duplication / memory allocation. But that's for the future I guess.

Only if absolute references are used.


> Other things that look a bit wasteful are:
>
>     ScFormulaCell*  pPrevious;
>     ScFormulaCell*  pNext;
>     ScFormulaCell*  pPreviousTrack;
>     ScFormulaCell*  pNextTrack;
>
> At least one pair of these (IIRC the 'Track') are needed only
> transiently during calculation as an append-only list and could be
> reasonably easily replaced by a bool bit-field and a std::vector on the
> ScDocument itself - which would save 48Mb or so (on 64bit).
True, a cell can be only in either the Track or the Tree, not both, so
reserving 4 pointers for this is waste now.

A vector would be most awkward as removing cells from any position and
growing/shrinking the track and tree frequently is necessary, so
a double linked list for each, the Track and the Tree, probably would be
best. Which per cell would save 2 pointers and add 2 booleans, given
memory alignment we might save 4 to 7 bytes per cell and had to rewrite
the tracking and cell maintenance code and access track/tree elements
through the container's iterators with performance penalty.

I'd rather get rid of 2 pointers and add 2 booleans and not add any
container to the document, which still would save 4 to 7 bytes per cell,
until/unless we're going to implement least recalculation paths in some
tree structure that could be calculated independently of each other.

  Eike

--
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GPG key "ID" 0x65632D3A - 2265 D7F3 A7B0 95CC 3918  630B 6A6C D5B7 6563 2D3A
Better use 64-bit 0x6A6CD5B765632D3A here is why: https://evil32.com/
Care about Free Software, support the FSFE https://fsfe.org/support/?erack

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

attachment0 (836 bytes) Download Attachment
Kohei Yoshida-7 Kohei Yoshida-7
Reply | Threaded
Open this post in threaded view
|

Re: ScFormulaCell size foo ...

Hi Eike,

On Fri, 2015-01-02 at 19:30 +0100, Eike Rathke wrote:
> If the ScFormulaCell instances were still allocated from the shared
> mempool we'd also have slightly less allocation overhead with the
> current code, but that is unrelated to the observation made here.
> Since
> introduction of #ifdef USE_MEMPOOL in sc/inc/formulacell.hxx and
> sc/source/core/data/formulacell.cxx the shared mempool is disabled.
> Why anyway?

I don't know who this question is directed at, but in case it's me, I
don't remember why, and if you believe enabling it would improve the
memory foot print, I'm all for it.  If there was any reason why, it
wouldn't be anything significant anyway.

Kohei

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice