[Libreoffice-commits] core.git: connectivity/source include/tools tools/source
Noel Grandin
noel at peralex.com
Thu Oct 9 01:18:47 PDT 2014
connectivity/source/drivers/dbase/DIndexIter.cxx | 6 -
connectivity/source/drivers/dbase/dindexnode.cxx | 58 ++++++++++---------
connectivity/source/inc/dbase/dindexnode.hxx | 70 +++++++++++++++--------
include/tools/ref.hxx | 13 +++-
tools/source/ref/ref.cxx | 10 ---
5 files changed, 93 insertions(+), 64 deletions(-)
New commits:
commit a0a919d2b541c415ad9b81d2ee91895bf106e9bb
Author: Noel Grandin <noel at peralex.com>
Date: Fri Oct 3 10:39:28 2014 +0200
remove SvRefBase::QueryDelete
Move it's functionality into the only place that needs it, in the dbase
driver.
Removes an extra virtual call from a widely used class.
The dbase driver seems to be using to perform some kind of whacky object
recycling, so it's not like we want this functionality to be used
somewhere else.
Change-Id: I41018f71e0b0a79fdd3d527536f0ac95c788e614
Reviewed-on: https://gerrit.libreoffice.org/11786
Reviewed-by: Noel Grandin <noelgrandin at gmail.com>
Tested-by: Noel Grandin <noelgrandin at gmail.com>
diff --git a/connectivity/source/drivers/dbase/DIndexIter.cxx b/connectivity/source/drivers/dbase/DIndexIter.cxx
index b1c0a17..edc3c99 100644
--- a/connectivity/source/drivers/dbase/DIndexIter.cxx
+++ b/connectivity/source/drivers/dbase/DIndexIter.cxx
@@ -52,7 +52,7 @@ sal_uIntPtr OIndexIterator::Find(bool bFirst)
if (bFirst)
{
m_aRoot = m_pIndex->getRoot();
- m_aCurLeaf = NULL;
+ m_aCurLeaf.Clear();
}
if (!m_pOperator)
@@ -186,7 +186,7 @@ sal_uIntPtr OIndexIterator::GetCompare(bool bFirst)
if ( ( ( pKey = GetNextKey() ) == NULL ) || !m_pOperator->operate(pKey,m_pOperand))
{
pKey = NULL;
- m_aCurLeaf = NULL;
+ m_aCurLeaf.Clear();
}
break;
case SQLFilterOperator::GREATER_EQUAL:
@@ -235,7 +235,7 @@ sal_uIntPtr OIndexIterator::GetNull(bool bFirst)
if ( ( ( pKey = GetNextKey() ) == NULL ) || !pKey->getValue().isNull())
{
pKey = NULL;
- m_aCurLeaf = NULL;
+ m_aCurLeaf.Clear();
}
return pKey ? pKey->GetRecord() : NODE_NOTFOUND;
}
diff --git a/connectivity/source/drivers/dbase/dindexnode.cxx b/connectivity/source/drivers/dbase/dindexnode.cxx
index 493ccd5..fe20627 100644
--- a/connectivity/source/drivers/dbase/dindexnode.cxx
+++ b/connectivity/source/drivers/dbase/dindexnode.cxx
@@ -86,6 +86,15 @@ ONDXPage::~ONDXPage()
delete[] ppNodes;
}
+void ONDXPage::ReleaseRef()
+{
+ assert( nRefCount >= 1);
+ if(--nRefCount == 0 && !bNoDelete)
+ {
+ QueryDelete();
+ }
+}
+
void ONDXPage::QueryDelete()
{
// Store in GarbageCollector
@@ -105,14 +114,20 @@ void ONDXPage::QueryDelete()
ppNodes[i] = ONDXNode();
}
- RestoreNoDelete();
+ bNoDelete = 1;
nCount = 0;
aParent.Clear();
rIndex.Collect(this);
}
else
- SvRefBase::QueryDelete();
+ {
+ // I'm not sure about the original purpose of this line, but right now
+ // it serves the purpose that anything that attempts to do an AddFirstRef()
+ // after an object is deleted will trip an assert.
+ nRefCount = 1 << 30;
+ delete this;
+ }
}
ONDXPagePtr& ONDXPage::GetChild(ODbaseIndex* pIndex)
@@ -189,7 +204,7 @@ bool ONDXPage::Insert(ONDXNode& rNode, sal_uInt32 nRowsLeft)
{
// this practically reduces the number of nodes by 1
- if (IsLeaf() && this == &rIndex.m_aCurLeaf)
+ if (IsLeaf() && this == rIndex.m_aCurLeaf)
{
// assumes, that the node, for which the condition (<=) holds, is stored in m_nCurNode
--nCount; // (otherwise we might get Assertions and GPFs - 60593)
@@ -344,7 +359,7 @@ void ONDXPage::Release(bool bSave)
ppNodes[i].GetChild().Clear();
}
- aParent = NULL;
+ aParent.Clear();
}
void ONDXPage::ReleaseFull(bool bSave)
@@ -487,7 +502,7 @@ void ONDXPage::Merge(sal_uInt16 nParentNodePos, ONDXPagePtr xPage)
sal_uInt16 nLastNode = bRight ? Count() - 1 : xPage->Count() - 1;
if (bRight)
{
- DBG_ASSERT(&xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
+ DBG_ASSERT(xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
// shift all nodes from xPage to the left node (append)
while (xPage->Count())
{
@@ -497,7 +512,7 @@ void ONDXPage::Merge(sal_uInt16 nParentNodePos, ONDXPagePtr xPage)
}
else
{
- DBG_ASSERT(&xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
+ DBG_ASSERT(xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
// xPage is the left page and THIS the right one
while (xPage->Count())
{
@@ -520,7 +535,7 @@ void ONDXPage::Merge(sal_uInt16 nParentNodePos, ONDXPagePtr xPage)
{
(*aParent)[0].SetChild();
aParent->ReleaseFull();
- aParent = NULL;
+ aParent.Clear();
rIndex.SetRootPos(nPagePos);
rIndex.m_aRoot = this;
SetModified(true);
@@ -567,7 +582,7 @@ void ONDXPage::Merge(sal_uInt16 nParentNodePos, ONDXPagePtr xPage)
{
if (bRight)
{
- DBG_ASSERT(&xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
+ DBG_ASSERT(xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
// Parent node will be integrated; is initialized with Child from xPage
(*aParent)[nParentNodePos].SetChild(xPage->GetChild(),aParent);
Append((*aParent)[nParentNodePos]);
@@ -576,7 +591,7 @@ void ONDXPage::Merge(sal_uInt16 nParentNodePos, ONDXPagePtr xPage)
}
else
{
- DBG_ASSERT(&xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
+ DBG_ASSERT(xPage != this,"xPage und THIS duerfen nicht gleich sein: Endlosschleife");
// Parent-node will be integrated; is initialized with child
(*aParent)[nParentNodePos].SetChild(GetChild(),aParent); // Parent memorizes my child
Insert(0,(*aParent)[nParentNodePos]); // insert parent node into myself
@@ -601,7 +616,7 @@ void ONDXPage::Merge(sal_uInt16 nParentNodePos, ONDXPagePtr xPage)
{
(*aParent).SetChild();
aParent->ReleaseFull();
- aParent = NULL;
+ aParent.Clear();
rIndex.SetRootPos(nPagePos);
rIndex.m_aRoot = this;
SetModified(true);
@@ -802,35 +817,24 @@ SvStream& connectivity::dbase::WriteONDXPagePtr(SvStream &rStream, const ONDXPag
ONDXPagePtr::ONDXPagePtr(const ONDXPagePtr& rRef)
- :ONDXPageRef(rRef)
+ :mpPage(rRef.mpPage)
,nPagePos(rRef.nPagePos)
{
+ if (mpPage != 0)
+ mpPage->AddNextRef();
}
ONDXPagePtr::ONDXPagePtr(ONDXPage* pRefPage)
- :ONDXPageRef(pRefPage)
+ :mpPage(pRefPage)
,nPagePos(0)
{
+ if (mpPage != 0)
+ mpPage->AddFirstRef();
if (pRefPage)
nPagePos = pRefPage->GetPagePos();
}
-ONDXPagePtr& ONDXPagePtr::operator=(const ONDXPagePtr& rRef)
-{
- ONDXPageRef::operator=(rRef);
- nPagePos = rRef.nPagePos;
- return *this;
-}
-
-
-ONDXPagePtr& ONDXPagePtr::operator= (ONDXPage* pRef)
-{
- ONDXPageRef::operator=(pRef);
- nPagePos = (pRef) ? pRef->GetPagePos() : 0;
- return *this;
-}
-
static sal_uInt32 nValue;
SvStream& connectivity::dbase::operator >> (SvStream &rStream, ONDXPage& rPage)
diff --git a/connectivity/source/inc/dbase/dindexnode.hxx b/connectivity/source/inc/dbase/dindexnode.hxx
index c59da5d..791df02 100644
--- a/connectivity/source/inc/dbase/dindexnode.hxx
+++ b/connectivity/source/inc/dbase/dindexnode.hxx
@@ -23,7 +23,6 @@
#include "file/FTable.hxx"
#include <connectivity/FValue.hxx>
#include <rtl/ref.hxx>
-#include <tools/ref.hxx>
#include <tools/stream.hxx>
#include <vector>
@@ -84,49 +83,53 @@ namespace connectivity
- // Index Page Pointer
-
class ONDXPage;
- typedef tools::SvRef<ONDXPage> ONDXPageRef; // Base class - because we need to store additional information
-
- class ONDXPagePtr : public ONDXPageRef
+ // Index Page Pointer
+ // This is ref-count pointer class
+ class ONDXPagePtr
{
friend SvStream& WriteONDXPagePtr(SvStream &rStream, const ONDXPagePtr&);
friend SvStream& operator >> (SvStream &rStream, ONDXPagePtr&);
+ ONDXPage* mpPage;
sal_uInt32 nPagePos; // Position in the index file
-
public:
- ONDXPagePtr(sal_uInt32 nPos = 0):nPagePos(nPos){}
+ ONDXPagePtr(sal_uInt32 nPos = 0) : mpPage(0), nPagePos(nPos) {}
ONDXPagePtr(const ONDXPagePtr& rRef);
ONDXPagePtr(ONDXPage* pRefPage);
-
- ONDXPagePtr& operator=(const ONDXPagePtr& rRef);
- ONDXPagePtr& operator=(ONDXPage* pPageRef);
+ inline ~ONDXPagePtr();
sal_uInt32 GetPagePos() const {return nPagePos;}
bool HasPage() const {return nPagePos != 0;}
- // sal_Bool Is() const { return isValid(); }
+
+ operator ONDXPage *() const { return mpPage; }
+ ONDXPage * operator ->() const { assert(mpPage != 0); return mpPage; }
+ bool Is() const { return mpPage != 0; }
+ inline void Clear();
};
// Index Page
-
- class ONDXPage : public SvRefBase
+ // This is a ref-counted class, with re-cycling
+ class ONDXPage
{
friend class ODbaseIndex;
+ friend class ONDXPagePtr;
friend SvStream& WriteONDXPage(SvStream &rStream, const ONDXPage&);
friend SvStream& operator >> (SvStream &rStream, ONDXPage&);
+ // the only reason this is not bool is because MSVC cannot handle mixed type bitfields
+ unsigned int bNoDelete : 1;
+ unsigned int nRefCount : 31;
sal_uInt32 nPagePos; // Position in the index file
- bool bModified : 1;
+ bool bModified : 1;
sal_uInt16 nCount;
- ONDXPagePtr aParent, // Parent page
- aChild; // Pointer to the right child page
- ODbaseIndex& rIndex;
- ONDXNode* ppNodes; // Array of nodes
+ ONDXPagePtr aParent, // Parent page
+ aChild; // Pointer to the right child page
+ ODbaseIndex& rIndex;
+ ONDXNode* ppNodes; // Array of nodes
public:
// Node operations
@@ -174,9 +177,22 @@ namespace connectivity
protected:
ONDXPage(ODbaseIndex& rIndex, sal_uInt32 nPos, ONDXPage* = NULL);
- virtual ~ONDXPage();
-
- virtual void QueryDelete() SAL_OVERRIDE;
+ ~ONDXPage();
+
+ void ReleaseRef();
+ void QueryDelete();
+ void AddNextRef()
+ {
+ assert( nRefCount < (1 << 30) && "Do not add refs to dead objects" );
+ ++nRefCount;
+ }
+ void AddFirstRef()
+ {
+ assert( nRefCount < (1 << 30) && "Do not add refs to dead objects" );
+ if( bNoDelete )
+ bNoDelete = 0;
+ ++nRefCount;
+ }
void SetModified(bool bMod) {bModified = bMod;}
void SetPagePos(sal_uInt32 nPage) {nPagePos = nPage;}
@@ -189,6 +205,16 @@ namespace connectivity
#endif
};
+ inline ONDXPagePtr::~ONDXPagePtr() { if (mpPage != 0) mpPage->ReleaseRef(); }
+ inline void ONDXPagePtr::Clear()
+ {
+ if (mpPage != 0) {
+ ONDXPage * pRefObj = mpPage;
+ mpPage = 0;
+ pRefObj->ReleaseRef();
+ }
+ }
+
SvStream& WriteONDXPagePtr(SvStream &rStream, const ONDXPagePtr&);
SvStream& operator >> (SvStream &rStream, ONDXPagePtr&);
diff --git a/include/tools/ref.hxx b/include/tools/ref.hxx
index b18ce05..aecc2b3 100644
--- a/include/tools/ref.hxx
+++ b/include/tools/ref.hxx
@@ -26,6 +26,10 @@
#include <tools/toolsdllapi.h>
#include <vector>
+/**
+ This implements similar functionality to boost::intrusive_ptr
+*/
+
namespace tools {
/** T must be a class that extends SvRefBase */
@@ -152,7 +156,6 @@ class TOOLS_DLLPUBLIC SvRefBase
protected:
virtual ~SvRefBase();
- virtual void QueryDelete();
public:
SvRefBase() : bNoDelete(1), nRefCount(0) {}
@@ -183,7 +186,13 @@ public:
{
assert( nRefCount >= 1);
if( --nRefCount == 0 && !bNoDelete)
- QueryDelete();
+ {
+ // I'm not sure about the original purpose of this line, but right now
+ // it serves the purpose that anything that attempts to do an AddRef()
+ // after an object is deleted will trip an assert.
+ nRefCount = 1 << 30;
+ delete this;
+ }
}
unsigned int GetRefCount() const
diff --git a/tools/source/ref/ref.cxx b/tools/source/ref/ref.cxx
index 3a6aa59..8be92d6 100644
--- a/tools/source/ref/ref.cxx
+++ b/tools/source/ref/ref.cxx
@@ -23,14 +23,4 @@ SvRefBase::~SvRefBase()
{
}
-void SvRefBase::QueryDelete()
-{
- bNoDelete = 0;
- // I'm not sure about the original purpose of this line, but right now
- // it serves the purpose that anything that attempts to do an AddRef()
- // after an object is deleted will trip an assert.
- nRefCount = 1 << 30;
- delete this;
-}
-
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
More information about the Libreoffice-commits
mailing list