[Libreoffice-commits] core.git: Branch 'private/kohei/calc-shared-string' - editeng/source include/svl sc/source svl/Library_svl.mk svl/qa svl/source

Kohei Yoshida kohei.yoshida at collabora.com
Mon Oct 7 09:15:30 PDT 2013


 editeng/source/editeng/editobj.cxx     |    2 
 include/svl/sharedstring.hxx           |   41 ++++++++++++++++
 include/svl/sharedstringpool.hxx       |    9 +--
 sc/source/core/data/column2.cxx        |    8 +--
 sc/source/core/data/column3.cxx        |    4 -
 sc/source/core/data/documentimport.cxx |    2 
 svl/Library_svl.mk                     |    1 
 svl/qa/unit/svl.cxx                    |   76 +++++++++++++-----------------
 svl/source/misc/sharedstring.cxx       |   81 +++++++++++++++++++++++++++++++++
 svl/source/misc/sharedstringpool.cxx   |   31 +++++++-----
 10 files changed, 187 insertions(+), 68 deletions(-)

New commits:
commit e5aea16b5ac9ce59e794cae69ec9a0984dc8cf74
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Mon Oct 7 12:15:28 2013 -0400

    Re-implement interning in order to return both string arrays.
    
    One is for the cased string and the other one for the non-cased one.
    
    Change-Id: I798687f2efecaaea73a09e0b3348f85a9d9e8c07

diff --git a/editeng/source/editeng/editobj.cxx b/editeng/source/editeng/editobj.cxx
index 3a5d41c..d1e1a6e 100644
--- a/editeng/source/editeng/editobj.cxx
+++ b/editeng/source/editeng/editobj.cxx
@@ -151,7 +151,7 @@ ContentInfo::~ContentInfo()
 
 void ContentInfo::NormalizeString( svl::SharedStringPool& rPool )
 {
-    aText = OUString(rPool.intern(aText));
+    aText = OUString(rPool.intern(aText).getData());
 }
 
 sal_uIntPtr ContentInfo::GetStringID( const svl::SharedStringPool& rPool ) const
diff --git a/include/svl/sharedstring.hxx b/include/svl/sharedstring.hxx
new file mode 100644
index 0000000..062d064
--- /dev/null
+++ b/include/svl/sharedstring.hxx
@@ -0,0 +1,41 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#ifndef SVL_SHAREDSTRING_HXX
+#define SVL_SHAREDSTRING_HXX
+
+#include "svl/svldllapi.h"
+#include "rtl/ustring.hxx"
+
+namespace svl {
+
+class SVL_DLLPUBLIC SharedString
+{
+    rtl_uString* mpData;
+    rtl_uString* mpDataIgnoreCase;
+public:
+    SharedString();
+    SharedString( rtl_uString* pData, rtl_uString* pDataIgnoreCase );
+    SharedString( const SharedString& r );
+    ~SharedString();
+
+    SharedString& operator= ( const SharedString& r );
+
+    rtl_uString* getData();
+    const rtl_uString* getData() const;
+
+    rtl_uString* getDataIgnoreCase();
+    const rtl_uString* getDataIgnoreCase() const;
+};
+
+}
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/include/svl/sharedstringpool.hxx b/include/svl/sharedstringpool.hxx
index 6793162..35ce28e 100644
--- a/include/svl/sharedstringpool.hxx
+++ b/include/svl/sharedstringpool.hxx
@@ -10,8 +10,7 @@
 #ifndef SVL_STRINGPOOL_HXX
 #define SVL_STRINGPOOL_HXX
 
-#include "svl/svldllapi.h"
-#include "rtl/ustring.hxx"
+#include "svl/sharedstring.hxx"
 
 #include <boost/unordered_map.hpp>
 #include <boost/unordered_set.hpp>
@@ -29,11 +28,11 @@ class SVL_DLLPUBLIC SharedStringPool
 {
     typedef boost::unordered_set<OUString, OUStringHash> StrHashType;
     typedef std::pair<StrHashType::iterator, bool> InsertResultType;
-    typedef boost::unordered_map<const rtl_uString*, OUString> StrIdMapType;
+    typedef boost::unordered_map<const rtl_uString*, OUString> StrStoreType;
 
     StrHashType maStrPool;
     StrHashType maStrPoolUpper;
-    StrIdMapType maToUpperMap;
+    StrStoreType maStrStore;
     const CharClass* mpCharClass;
 
 public:
@@ -49,7 +48,7 @@ public:
      * @return a pointer to the string object stored inside the pool, or NULL
      *         if the insertion fails.
      */
-    rtl_uString* intern( const OUString& rStr );
+    SharedString intern( const OUString& rStr );
 
     /**
      * Get a unique ID of string object that's expected to be in the shared
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 9601583..2b7d522 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -2174,7 +2174,7 @@ bool appendStrings(
                 getBlockIterators<sc::string_block>(it, nLenRemain, itData, itDataEnd);
 
                 for (; itData != itDataEnd; ++itData)
-                    rArray.push_back(rCxt.maStrPool.intern(*itData));
+                    rArray.push_back(rCxt.maStrPool.intern(*itData).getData());
             }
             break;
             case sc::element_type_edittext:
@@ -2185,7 +2185,7 @@ bool appendStrings(
                 for (; itData != itDataEnd; ++itData)
                 {
                     OUString aStr = ScEditUtil::GetString(**itData, pDoc);
-                    rArray.push_back(rCxt.maStrPool.intern(aStr));
+                    rArray.push_back(rCxt.maStrPool.intern(aStr).getData());
                 }
             }
             break;
@@ -2210,7 +2210,7 @@ bool appendStrings(
                         return false;
                     }
 
-                    rArray.push_back(rCxt.maStrPool.intern(aStr));
+                    rArray.push_back(rCxt.maStrPool.intern(aStr).getData());
                 }
             }
             break;
@@ -2250,7 +2250,7 @@ void copyFirstBlock( sc::FormulaGroupContext& rCxt, size_t nLen, const sc::CellS
     const OUString* p = &sc::string_block::at(*rPos.first->data, rPos.second);
     const OUString* pEnd = p + nLen;
     for (; p != pEnd; ++p)
-        rArray.push_back(rCxt.maStrPool.intern(*p));
+        rArray.push_back(rCxt.maStrPool.intern(*p).getData());
 }
 
 }
diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index c651cca..43486b1 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -2188,7 +2188,7 @@ void ScColumn::SetRawString( SCROW nRow, const OUString& rStr, bool bBroadcast )
     if (!ValidRow(nRow))
         return;
 
-    rtl_uString* pStr = pDocument->GetCellStringPool().intern(rStr);
+    rtl_uString* pStr = pDocument->GetCellStringPool().intern(rStr).getData();
     if (!pStr)
         return;
 
@@ -2207,7 +2207,7 @@ void ScColumn::SetRawString(
     if (!ValidRow(nRow))
         return;
 
-    rtl_uString* pStr = pDocument->GetCellStringPool().intern(rStr);
+    rtl_uString* pStr = pDocument->GetCellStringPool().intern(rStr).getData();
     if (!pStr)
         return;
 
diff --git a/sc/source/core/data/documentimport.cxx b/sc/source/core/data/documentimport.cxx
index 4822557..95ae3cf 100644
--- a/sc/source/core/data/documentimport.cxx
+++ b/sc/source/core/data/documentimport.cxx
@@ -150,7 +150,7 @@ void ScDocumentImport::setStringCell(const ScAddress& rPos, const OUString& rStr
     if (!pBlockPos)
         return;
 
-    rtl_uString* pStr = mpImpl->mrDoc.GetCellStringPool().intern(rStr);
+    rtl_uString* pStr = mpImpl->mrDoc.GetCellStringPool().intern(rStr).getData();
     if (!pStr)
         return;
 
diff --git a/svl/Library_svl.mk b/svl/Library_svl.mk
index dc2bf41..283e06b 100644
--- a/svl/Library_svl.mk
+++ b/svl/Library_svl.mk
@@ -110,6 +110,7 @@ $(eval $(call gb_Library_add_exception_objects,svl,\
     svl/source/misc/lockfilecommon \
     svl/source/misc/ownlist \
     svl/source/misc/sharecontrolfile \
+    svl/source/misc/sharedstring \
     svl/source/misc/sharedstringpool \
     svl/source/misc/strmadpt \
     svl/source/misc/urihelper \
diff --git a/svl/qa/unit/svl.cxx b/svl/qa/unit/svl.cxx
index fe987ec..4bd6313 100644
--- a/svl/qa/unit/svl.cxx
+++ b/svl/qa/unit/svl.cxx
@@ -301,42 +301,32 @@ void Test::testStringPool()
     SvtSysLocale aSysLocale;
     svl::SharedStringPool aPool(aSysLocale.GetCharClassPtr());
 
-    const rtl_uString* p1 = aPool.intern("Andy");
-    const rtl_uString* p2 = aPool.intern("Andy");
-    CPPUNIT_ASSERT_EQUAL(p1, p2);
+    svl::SharedString p1, p2;
+    p1 = aPool.intern("Andy");
+    p2 = aPool.intern("Andy");
+    CPPUNIT_ASSERT_EQUAL(p1.getData(), p2.getData());
 
     p2 = aPool.intern("Bruce");
-    CPPUNIT_ASSERT_MESSAGE("They must differ.", p1 != p2);
+    CPPUNIT_ASSERT_MESSAGE("They must differ.", p1.getData() != p2.getData());
 
     OUString aAndy("Andy");
-    sal_uIntPtr si1 = aPool.getIdentifier("Andy");
-    sal_uIntPtr si2 = aPool.getIdentifier(aAndy);
-    CPPUNIT_ASSERT_MESSAGE("Identifier shouldn't be 0.", si1);
-    CPPUNIT_ASSERT_MESSAGE("Identifier shouldn't be 0.", si2);
-    CPPUNIT_ASSERT_EQUAL(si1, si2);
-    si1 = aPool.getIdentifierIgnoreCase(aAndy);
-    CPPUNIT_ASSERT_MESSAGE("Case insensitive identifier shouldn't be 0.", si1);
+    p1 = aPool.intern("Andy");
+    p2 = aPool.intern(aAndy);
+    CPPUNIT_ASSERT_MESSAGE("Identifier shouldn't be NULL.", p1.getData());
+    CPPUNIT_ASSERT_MESSAGE("Identifier shouldn't be NULL.", p2.getData());
+    CPPUNIT_ASSERT_EQUAL(p1.getData(), p2.getData());
 
     // Test case insensitive string ID's.
     OUString aAndyLower("andy"), aAndyUpper("ANDY");
-    si1 = aPool.getIdentifier("Andy");
-    CPPUNIT_ASSERT_MESSAGE("This shouldn't be NULL.", si1);
-    aPool.intern(aAndyLower);
-    si2 = aPool.getIdentifier(aAndyLower);
-    CPPUNIT_ASSERT_MESSAGE("They must differ.", si1 != si2);
-    aPool.intern(aAndyUpper);
-    si2 = aPool.getIdentifier(aAndyUpper);
-    CPPUNIT_ASSERT_MESSAGE("They must differ.", si1 != si2);
-
-    si1 = aPool.getIdentifierIgnoreCase("Andy");
-    CPPUNIT_ASSERT_MESSAGE("This shouldn't be NULL.", si1);
-    si2 = aPool.getIdentifierIgnoreCase("andy");
-    CPPUNIT_ASSERT_MESSAGE("This shouldn't be NULL.", si2);
-    CPPUNIT_ASSERT_EQUAL(si1, si2);
-
-    si2 = aPool.getIdentifierIgnoreCase("ANDY");
-    CPPUNIT_ASSERT_MESSAGE("This shouldn't be NULL.", si2);
-    CPPUNIT_ASSERT_EQUAL(si1, si2);
+    p1 = aPool.intern(aAndy);
+    p2 = aPool.intern(aAndyLower);
+    CPPUNIT_ASSERT_MESSAGE("Failed to intern strings.", p1.getData() && p2.getData());
+    CPPUNIT_ASSERT_MESSAGE("These two ID's should differ.", p1.getData() != p2.getData());
+    CPPUNIT_ASSERT_MESSAGE("These two ID's should be equal.", p1.getDataIgnoreCase() == p2.getDataIgnoreCase());
+    p2 = aPool.intern(aAndyUpper);
+    CPPUNIT_ASSERT_MESSAGE("Failed to intern string.", p2.getData());
+    CPPUNIT_ASSERT_MESSAGE("These two ID's should differ.", p1.getData() != p2.getData());
+    CPPUNIT_ASSERT_MESSAGE("These two ID's should be equal.", p1.getDataIgnoreCase() == p2.getDataIgnoreCase());
 }
 
 void Test::testStringPoolPurge()
@@ -353,8 +343,8 @@ void Test::testStringPoolPurge()
     // Since no string objects referencing the pooled strings exist, purging
     // the pool should empty it.
     aPool.purge();
-    CPPUNIT_ASSERT_MESSAGE("Wrong string count.", aPool.getCount() == 0);
-    CPPUNIT_ASSERT_MESSAGE("Wrong case insensitive string count.", aPool.getCountIgnoreCase() == 0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCountIgnoreCase());
 
     // Now, create string objects on the heap.
     boost::scoped_ptr<OUString> pStr1(new OUString("Andy"));
@@ -366,37 +356,37 @@ void Test::testStringPoolPurge()
     aPool.intern(*pStr3);
     aPool.intern(*pStr4);
 
-    CPPUNIT_ASSERT_MESSAGE("Wrong string count.", aPool.getCount() == 4);
-    CPPUNIT_ASSERT_MESSAGE("Wrong case insensitive string count.", aPool.getCountIgnoreCase() == 2);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
 
     // This shouldn't purge anything.
     aPool.purge();
-    CPPUNIT_ASSERT_MESSAGE("Wrong string count.", aPool.getCount() == 4);
-    CPPUNIT_ASSERT_MESSAGE("Wrong case insensitive string count.", aPool.getCountIgnoreCase() == 2);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aPool.getCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
 
     // Delete one heap string object, and purge. That should purge one string.
     pStr1.reset();
     aPool.purge();
-    CPPUNIT_ASSERT_MESSAGE("Wrong string count.", aPool.getCount() == 3);
-    CPPUNIT_ASSERT_MESSAGE("Wrong case insensitive string count.", aPool.getCountIgnoreCase() == 2);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), aPool.getCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
 
     // Ditto...
     pStr3.reset();
     aPool.purge();
-    CPPUNIT_ASSERT_MESSAGE("Wrong string count.", aPool.getCount() == 2);
-    CPPUNIT_ASSERT_MESSAGE("Wrong case insensitive string count.", aPool.getCountIgnoreCase() == 2);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aPool.getCountIgnoreCase());
 
     // Again.
     pStr2.reset();
     aPool.purge();
-    CPPUNIT_ASSERT_MESSAGE("Wrong string count.", aPool.getCount() == 1);
-    CPPUNIT_ASSERT_MESSAGE("Wrong case insensitive string count.", aPool.getCountIgnoreCase() == 1);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aPool.getCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aPool.getCountIgnoreCase());
 
     // Delete 'Bruce' and purge.
     pStr4.reset();
     aPool.purge();
-    CPPUNIT_ASSERT_MESSAGE("Wrong string count.", aPool.getCount() == 0);
-    CPPUNIT_ASSERT_MESSAGE("Wrong case insensitive string count.", aPool.getCountIgnoreCase() == 0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), aPool.getCountIgnoreCase());
 }
 
 void Test::checkPreviewString(SvNumberFormatter& aFormatter,
diff --git a/svl/source/misc/sharedstring.cxx b/svl/source/misc/sharedstring.cxx
new file mode 100644
index 0000000..d5b27bd
--- /dev/null
+++ b/svl/source/misc/sharedstring.cxx
@@ -0,0 +1,81 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include "svl/sharedstring.hxx"
+
+namespace svl {
+
+SharedString::SharedString() : mpData(NULL), mpDataIgnoreCase(NULL) {}
+
+SharedString::SharedString( rtl_uString* pData, rtl_uString* pDataIgnoreCase ) :
+    mpData(pData), mpDataIgnoreCase(pDataIgnoreCase)
+{
+    if (mpData)
+        rtl_uString_acquire(mpData);
+    if (mpDataIgnoreCase)
+        rtl_uString_acquire(mpDataIgnoreCase);
+}
+
+SharedString::SharedString( const SharedString& r ) : mpData(r.mpData), mpDataIgnoreCase(r.mpDataIgnoreCase)
+{
+    if (mpData)
+        rtl_uString_acquire(mpData);
+    if (mpDataIgnoreCase)
+        rtl_uString_acquire(mpDataIgnoreCase);
+}
+
+SharedString::~SharedString()
+{
+    if (mpData)
+        rtl_uString_release(mpData);
+    if (mpDataIgnoreCase)
+        rtl_uString_release(mpDataIgnoreCase);
+}
+
+SharedString& SharedString::operator= ( const SharedString& r )
+{
+    if (mpData)
+        rtl_uString_release(mpData);
+    if (mpDataIgnoreCase)
+        rtl_uString_release(mpDataIgnoreCase);
+
+    mpData = r.mpData;
+    mpDataIgnoreCase = r.mpDataIgnoreCase;
+
+    if (mpData)
+        rtl_uString_acquire(mpData);
+    if (mpDataIgnoreCase)
+        rtl_uString_acquire(mpDataIgnoreCase);
+
+    return *this;
+}
+
+rtl_uString* SharedString::getData()
+{
+    return mpData;
+}
+
+const rtl_uString* SharedString::getData() const
+{
+    return mpData;
+}
+
+rtl_uString* SharedString::getDataIgnoreCase()
+{
+    return mpDataIgnoreCase;
+}
+
+const rtl_uString* SharedString::getDataIgnoreCase() const
+{
+    return mpDataIgnoreCase;
+}
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svl/source/misc/sharedstringpool.cxx b/svl/source/misc/sharedstringpool.cxx
index 805a6fc..6b8f152 100644
--- a/svl/source/misc/sharedstringpool.cxx
+++ b/svl/source/misc/sharedstringpool.cxx
@@ -15,21 +15,29 @@ namespace svl {
 SharedStringPool::SharedStringPool() : mpCharClass(NULL) {}
 SharedStringPool::SharedStringPool( const CharClass* pCharClass ) : mpCharClass(pCharClass) {}
 
-rtl_uString* SharedStringPool::intern( const OUString& rStr )
+SharedString SharedStringPool::intern( const OUString& rStr )
 {
     InsertResultType aRes = findOrInsert(maStrPool, rStr);
     if (aRes.first == maStrPool.end())
         // Insertion failed.
-        return NULL;
+        return SharedString();
 
     rtl_uString* pOrig = aRes.first->pData;
 
+    if (!mpCharClass)
+        // We don't track case insensitive strings.
+        return SharedString(pOrig, NULL);
+
     if (!aRes.second)
+    {
         // No new string has been inserted. Return the existing string in the pool.
-        return pOrig;
+        StrStoreType::iterator it = maStrStore.find(pOrig);
+        if (it == maStrStore.end())
+            return SharedString();
 
-    if (!mpCharClass)
-        return pOrig;
+        rtl_uString* pUpper = it->second.pData;
+        return SharedString(pOrig, pUpper);
+    }
 
     // This is a new string insertion. Establish mapping to upper-case variant.
 
@@ -37,12 +45,11 @@ rtl_uString* SharedStringPool::intern( const OUString& rStr )
     aRes = findOrInsert(maStrPoolUpper, aUpper);
     if (aRes.first == maStrPoolUpper.end())
         // Failed to insert or fetch upper-case variant. Should never happen.
-        return pOrig;
+        return SharedString();
 
-    // Set mapping.
-    maToUpperMap.insert(StrIdMapType::value_type(pOrig, *aRes.first));
+    maStrStore.insert(StrStoreType::value_type(pOrig, *aRes.first));
 
-    return pOrig;
+    return SharedString(pOrig, aRes.first->pData);
 }
 
 sal_uIntPtr SharedStringPool::getIdentifier( const OUString& rStr ) const
@@ -58,8 +65,8 @@ sal_uIntPtr SharedStringPool::getIdentifierIgnoreCase( const OUString& rStr ) co
         // Not in the pool.
         return 0;
 
-    StrIdMapType::const_iterator itUpper = maToUpperMap.find(itOrig->pData);
-    if (itUpper == maToUpperMap.end())
+    StrStoreType::const_iterator itUpper = maStrStore.find(itOrig->pData);
+    if (itUpper == maStrStore.end())
         // Passed string is not in the pool.
         return 0;
 
@@ -87,7 +94,7 @@ void SharedStringPool::purge()
         {
             // Remove it from the upper string map.  This should unref the
             // upper string linked to this original string.
-            maToUpperMap.erase(p);
+            maStrStore.erase(p);
         }
         else
             // Still referenced outside the pool. Keep it.


More information about the Libreoffice-commits mailing list