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.me...@collabora.com <><, Pseudo Engineer, itinerant idiot
From 952bdaf75b996e2cbf86a205e5d944c2f50c7ca1 Mon Sep 17 00:00:00 2001
From: Michael Meeks <michael.me...@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
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
