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.

-- 
 michael.meeks@collabora.com  <><, Pseudo Engineer, itinerant idiot

From 952bdaf75b996e2cbf86a205e5d944c2f50c7ca1 Mon Sep 17 00:00:00 2001
From: Michael Meeks <michael.meeks@collabora.com>
Date: Tue, 23 Dec 2014 09:47:59 +0000
Subject: [PATCH] svl: Make SvtListener more efficient when it has no
 listeners.

boost::unordered_set has a large in-line size: 56 bytes or so and
with the new formula group dependency work, this burns ~30% of the
size of ScFormulaCell which is 20% of our total memory usage. So
instead allocate that as needed on the heap.

Change-Id: I17786516f27e27b4cbc31306bccdaf5dd1f3d017
---
 include/svl/listener.hxx       |  2 +-
 svl/source/notify/listener.cxx | 60 +++++++++++++++++++++++++++++++-----------
 2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/include/svl/listener.hxx b/include/svl/listener.hxx
index 8b47fda..93fd04e 100644
--- a/include/svl/listener.hxx
+++ b/include/svl/listener.hxx
@@ -29,7 +29,7 @@ class SfxHint;
 class SVL_DLLPUBLIC SvtListener
 {
     typedef boost::unordered_set<SvtBroadcaster*> BroadcastersType;
-    BroadcastersType maBroadcasters;
+    BroadcastersType *mpBroadcasters;
 
     const SvtListener&  operator=(const SvtListener &); // n.i., ist verboten
 
diff --git a/svl/source/notify/listener.cxx b/svl/source/notify/listener.cxx
index 9c2a392..3625a67 100644
--- a/svl/source/notify/listener.cxx
+++ b/svl/source/notify/listener.cxx
@@ -28,10 +28,15 @@ sal_uInt16 SvtListener::QueryBase::getId() const
     return mnId;
 }
 
-SvtListener::SvtListener() {}
+SvtListener::SvtListener() : mpBroadcasters(NULL) {}
 
-SvtListener::SvtListener( const SvtListener &r ) :
-    maBroadcasters(r.maBroadcasters) {}
+SvtListener::SvtListener( const SvtListener &r )
+{
+    if (r.mpBroadcasters)
+        mpBroadcasters = new BroadcastersType(*r.mpBroadcasters);
+    else
+        mpBroadcasters = NULL;
+}
 
 SvtListener::~SvtListener()
 {
@@ -43,8 +48,11 @@ SvtListener::~SvtListener()
 
 bool SvtListener::StartListening( SvtBroadcaster& rBroadcaster )
 {
+    if (!mpBroadcasters)
+        mpBroadcasters = new BroadcastersType();
+
     std::pair<BroadcastersType::iterator, bool> r =
-        maBroadcasters.insert(&rBroadcaster);
+        mpBroadcasters->insert(&rBroadcaster);
     if (r.second)
     {
         // This is a new broadcaster.
@@ -55,48 +63,70 @@ bool SvtListener::StartListening( SvtBroadcaster& rBroadcaster )
 
 bool SvtListener::EndListening( SvtBroadcaster& rBroadcaster )
 {
-    BroadcastersType::iterator it = maBroadcasters.find(&rBroadcaster);
-    if (it == maBroadcasters.end())
+    if (!mpBroadcasters)
+        return false;
+
+    BroadcastersType::iterator it = mpBroadcasters->find(&rBroadcaster);
+    if (it == mpBroadcasters->end())
         // Not listening to this broadcaster.
         return false;
 
     rBroadcaster.Remove(this);
-    maBroadcasters.erase(it);
+    mpBroadcasters->erase(it);
+    if (mpBroadcasters->size() == 0)
+    {
+        delete mpBroadcasters;
+        mpBroadcasters = NULL;
+    }
     return true;
 }
 
 void SvtListener::EndListeningAll()
 {
-    BroadcastersType::iterator it = maBroadcasters.begin(), itEnd = maBroadcasters.end();
+    if (!mpBroadcasters)
+        return;
+
+    BroadcastersType::iterator it = mpBroadcasters->begin(), itEnd = mpBroadcasters->end();
     for (; it != itEnd; ++it)
     {
         SvtBroadcaster& rBC = **it;
         rBC.Remove(this);
     }
-    maBroadcasters.clear();
+    delete mpBroadcasters;
+    mpBroadcasters = NULL;
 }
 
-
 bool SvtListener::IsListening( SvtBroadcaster& rBroadcaster ) const
 {
-    return maBroadcasters.count(&rBroadcaster) > 0;
+    return mpBroadcasters && mpBroadcasters->count(&rBroadcaster) > 0;
 }
 
 void SvtListener::CopyAllBroadcasters( const SvtListener& r )
 {
-    BroadcastersType aCopy(r.maBroadcasters);
-    maBroadcasters.swap(aCopy);
-    BroadcastersType::iterator it = maBroadcasters.begin(), itEnd = maBroadcasters.end();
+    if (!r.mpBroadcasters)
+    {
+        // FIXME: this path, before as now appears to fail
+        // to remove any existing listeners from registered
+        // broadcasters.
+        delete mpBroadcasters;
+        mpBroadcasters = NULL;
+        return;
+    }
+
+    BroadcastersType *pRelease = mpBroadcasters;
+    mpBroadcasters = new BroadcastersType(*r.mpBroadcasters);
+    BroadcastersType::iterator it = mpBroadcasters->begin(), itEnd = mpBroadcasters->end();
     for (; it != itEnd; ++it)
     {
         SvtBroadcaster* p = *it;
         p->Add(this);
     }
+    delete pRelease;
 }
 
 bool SvtListener::HasBroadcaster() const
 {
-    return !maBroadcasters.empty();
+    return mpBroadcasters && !mpBroadcasters->empty();
 }
 
 void SvtListener::Notify( const SfxHint& /*rHint*/ ) {}
-- 
1.8.4.5