Hi
So I've been working on this patch to move mpNotes from ScBaseCell to
ScTable, and I got it mostly working.
But I was struggling with leaks and getting the lifecycle of the notes
to exactly match the lifecycle of the ScBaseCell object.
So then I had a weird idea - how about if I tied the lifecycles
together implicitly?
So I came up with this patch.
It compiles, and passes a make check.
I'll do a memcheck run on Monday when I can get access to my other
machine with tons of memory.
Regards, Noel Grandin
diff --git a/sc/inc/cell.hxx b/sc/inc/cell.hxx
old mode 100644
new mode 100755
index fa1b719..1755591
--- a/sc/inc/cell.hxx
+++ b/sc/inc/cell.hxx
@@ -122,11 +122,11 @@ public:
inline void SetScriptType( sal_uInt8 nNew ) { nScriptType = nNew; }
/** Returns true, if the cell contains a note. */
- inline bool HasNote() const { return mpNote != 0; }
+ bool HasNote() const;
/** Returns the pointer to a cell note object (read-only). */
- inline const ScPostIt* GetNote() const { return mpNote; }
+ const ScPostIt* GetNote() const;
/** Returns the pointer to a cell note object. */
- inline ScPostIt* GetNote() { return mpNote; }
+ ScPostIt* GetNote();
/** Takes ownership of the passed cell note object. */
void TakeNote( ScPostIt* pNote );
/** Returns and forgets the own cell note object. Caller takes ownership!
*/
@@ -169,7 +169,6 @@ private:
ScBaseCell& operator=( const ScBaseCell& );
private:
- ScPostIt* mpNote; /// The cell note. Cell takes ownership!
SvtBroadcaster* mpBroadcaster; /// Broadcaster for changed values. Cell
takes ownership!
protected:
diff --git a/sc/source/core/data/cell.cxx b/sc/source/core/data/cell.cxx
old mode 100644
new mode 100755
index 6cc8209..04aae8c
--- a/sc/source/core/data/cell.cxx
+++ b/sc/source/core/data/cell.cxx
@@ -82,10 +82,16 @@ IMPL_FIXEDMEMPOOL_NEWDEL( ScStringCell,
nMemPoolStringCell, nMemPoolStringCell
IMPL_FIXEDMEMPOOL_NEWDEL( ScNoteCell, nMemPoolNoteCell, nMemPoolNoteCell )
#endif
+/**
+ * The notes map. Because only a handful of notes typically exist in an
application, but there can be millions
+ * of cells, we don't want to store this on the cell itself
+ */
+typedef ::std::map<const ScBaseCell*, ScPostIt*> ScNotesMap;
+static ScNotesMap notesMap;
+
// ============================================================================
ScBaseCell::ScBaseCell( CellType eNewType ) :
- mpNote( 0 ),
mpBroadcaster( 0 ),
nTextWidth( TEXTWIDTH_DIRTY ),
eCellType( sal::static_int_cast<sal_uInt8>(eNewType) ),
@@ -94,7 +100,6 @@ ScBaseCell::ScBaseCell( CellType eNewType ) :
}
ScBaseCell::ScBaseCell( const ScBaseCell& rCell ) :
- mpNote( 0 ),
mpBroadcaster( 0 ),
nTextWidth( rCell.nTextWidth ),
eCellType( rCell.eCellType ),
@@ -104,7 +109,12 @@ ScBaseCell::ScBaseCell( const ScBaseCell& rCell ) :
ScBaseCell::~ScBaseCell()
{
- delete mpNote;
+ ScNotesMap::iterator it = notesMap.find(this);
+ if (it != notesMap.end()) {
+ ScPostIt *pNote = it->second;
+ notesMap.erase(this);
+ delete pNote;
+ }
delete mpBroadcaster;
OSL_ENSURE( eCellType == CELLTYPE_DESTROYED, "BaseCell Destructor" );
}
@@ -244,12 +254,14 @@ ScBaseCell* ScBaseCell::CloneWithoutNote( ScDocument&
rDestDoc, const ScAddress&
ScBaseCell* ScBaseCell::CloneWithNote( const ScAddress& rOwnPos, ScDocument&
rDestDoc, const ScAddress& rDestPos, int nCloneFlags ) const
{
ScBaseCell* pNewCell = lclCloneCell( *this, rDestDoc, rDestPos,
nCloneFlags );
- if( mpNote )
+ ScNotesMap::iterator it = notesMap.find(this);
+ if (it != notesMap.end())
{
if( !pNewCell )
pNewCell = new ScNoteCell;
bool bCloneCaption = (nCloneFlags & SC_CLONECELL_NOCAPTION) == 0;
- pNewCell->TakeNote( mpNote->Clone( rOwnPos, rDestDoc, rDestPos,
bCloneCaption ) );
+ ScPostIt* pNote = it->second;
+ pNewCell->TakeNote( pNote->Clone( rOwnPos, rDestDoc, rDestPos,
bCloneCaption ) );
}
return pNewCell;
}
@@ -282,27 +294,63 @@ void ScBaseCell::Delete()
bool ScBaseCell::IsBlank( bool bIgnoreNotes ) const
{
- return (eCellType == CELLTYPE_NOTE) && (bIgnoreNotes || !mpNote);
+ return (eCellType == CELLTYPE_NOTE) && (bIgnoreNotes ||
notesMap.find(this) == notesMap.end());
+}
+
+/** Returns true, if the cell contains a note. */
+bool ScBaseCell::HasNote() const
+{
+ return notesMap.find(this) != notesMap.end();
+}
+
+/** Returns the pointer to a cell note object. */
+ScPostIt* ScBaseCell::GetNote()
+{
+ ScNotesMap::iterator it = notesMap.find(this);
+ return it == notesMap.end() ? 0 : it->second;
+}
+
+/** Returns the pointer to a cell note object (read-only). */
+const ScPostIt* ScBaseCell::GetNote() const
+{
+ ScNotesMap::iterator it = notesMap.find(this);
+ return it == notesMap.end() ? 0 : it->second;
}
void ScBaseCell::TakeNote( ScPostIt* pNote )
{
- delete mpNote;
- mpNote = pNote;
+ ScNotesMap::iterator it = notesMap.find(this);
+ if (it != notesMap.end())
+ {
+ delete it->second;
+ }
+ notesMap[this] = pNote;
}
ScPostIt* ScBaseCell::ReleaseNote()
{
- ScPostIt* pNote = mpNote;
- mpNote = 0;
- return pNote;
+ ScNotesMap::iterator it = notesMap.find(this);
+ if (it != notesMap.end())
+ {
+ ScPostIt* pNote = it->second;
+ notesMap.erase(it);
+ return pNote;
+ }
+ else
+ return 0;
}
void ScBaseCell::DeleteNote()
{
- DELETEZ( mpNote );
+ ScNotesMap::iterator it = notesMap.find(this);
+ if (it != notesMap.end())
+ {
+ ScPostIt* pNote = it->second;
+ notesMap.erase(it);
+ delete pNote;
+ }
}
-
+
void ScBaseCell::TakeBroadcaster( SvtBroadcaster* pBroadcaster )
{
delete mpBroadcaster;
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
