WhichRangesContainer, reduce malloc in SfxItemSet

SfxItemSet shows up in perf profiles frequently,
and the hottest part is the malloc of the two arrays we need.
But most of the time, one of those arrays is a compile-time
constant.

So this change introduces
(*) WhichRangesContainer, which manages whether the SfxItemSet
owns the array it points at or not.
(*) a static const member in svl::Items (idea from mkaganski)
to store the data.

Change-Id: Icb8cdbc4d54fd76739565c575e16a744515e5355
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118703
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

7 files changed, 41 insertions, 62 deletions

diff --git a/sfx2/source/appl/appcfg.cxx b/sfx2/source/appl/appcfg.cxx
index bac9a650966f..4595f6508ca0 100644
--- a/sfx2/source/appl/appcfg.cxx
+++ b/sfx2/source/appl/appcfg.cxx
@@ -114,15 +114,15 @@ void SfxApplication::GetOptions( SfxItemSet& rSet )
     bool bRet = false;
     SfxItemPool &rPool = GetPool();
 
-    const sal_uInt16 *pRanges = rSet.GetRanges();
+    const WhichRangesContainer& pRanges = rSet.GetRanges();
     SvtSaveOptions aSaveOptions;
     SvtHelpOptions aHelpOptions;
     SvtSecurityOptions  aSecurityOptions;
     SvtMiscOptions aMiscOptions;
 
-    while ( *pRanges )
+    for (auto const & pRange : pRanges)
     {
-        for(sal_uInt16 nWhich = *pRanges++; nWhich <= *pRanges; ++nWhich)
+        for(sal_uInt16 nWhich = pRange.first; nWhich <= pRange.second; ++nWhich)
         {
             switch(nWhich)
             {
@@ -390,7 +390,6 @@ void SfxApplication::GetOptions( SfxItemSet& rSet )
             }
             SAL_WARN_IF(!bRet, "sfx.appl", "Putting options failed!");
         }
-        pRanges++;
     }
 }
 
diff --git a/sfx2/source/appl/appserv.cxx b/sfx2/source/appl/appserv.cxx
index fe23dd1935f6..4e4e5f90c3a2 100644
--- a/sfx2/source/appl/appserv.cxx
+++ b/sfx2/source/appl/appserv.cxx
@@ -1080,11 +1080,11 @@ void SfxApplication::MiscExec_Impl( SfxRequest& rReq )
 
 void SfxApplication::MiscState_Impl(SfxItemSet &rSet)
 {
-    const sal_uInt16 *pRanges = rSet.GetRanges();
-    DBG_ASSERT(pRanges && *pRanges, "Set without range");
-    while ( *pRanges )
+    const WhichRangesContainer & pRanges = rSet.GetRanges();
+    DBG_ASSERT(!pRanges.empty(), "Set without range");
+    for ( auto const & pRange : pRanges )
     {
-        for(sal_uInt16 nWhich = *pRanges++; nWhich <= *pRanges; ++nWhich)
+        for(sal_uInt16 nWhich = pRange.first; nWhich <= pRange.second; ++nWhich)
         {
             switch(nWhich)
             {
@@ -1278,8 +1278,6 @@ void SfxApplication::MiscState_Impl(SfxItemSet &rSet)
                     break;
             }
         }
-
-        ++pRanges;
     }
 }
 
diff --git a/sfx2/source/appl/appuno.cxx b/sfx2/source/appl/appuno.cxx
index cc4917eebb11..17c2304ed10a 100644
--- a/sfx2/source/appl/appuno.cxx
+++ b/sfx2/source/appl/appuno.cxx
@@ -1118,11 +1118,10 @@ void TransformItems( sal_uInt16 nSlotId, const SfxItemSet& rSet, uno::Sequence<b
     if ( rSet.Count() != nItems )
     {
         // detect unknown item and present error message
-        const sal_uInt16 *pRanges = rSet.GetRanges();
-        while ( *pRanges )
+        for ( auto const & rPair : rSet.GetRanges() )
         {
-            sal_uInt16 nStartWhich = *pRanges++;
-            sal_uInt16 nEndWhich = *pRanges++;
+            sal_uInt16 nStartWhich = rPair.first;
+            sal_uInt16 nEndWhich = rPair.second;
             for(sal_uInt16 nId = nStartWhich; nId <= nEndWhich; ++nId)
             {
                 if ( rSet.GetItemState(nId) < SfxItemState::SET ) //???
diff --git a/sfx2/source/control/bindings.cxx b/sfx2/source/control/bindings.cxx
index ca8366f26539..042684647114 100644
--- a/sfx2/source/control/bindings.cxx
+++ b/sfx2/source/control/bindings.cxx
@@ -1166,8 +1166,8 @@ std::optional<SfxItemSet> SfxBindings::CreateSet_Impl
     }
 
     // Create a Set from the ranges
+    WhichRangesContainer ranges;
     size_t i = 0;
-    SfxItemSet aSet(rPool, nullptr);
     while ( i < rFound.size() )
     {
         const sal_uInt16 nWhich1 = rFound[i].nWhichId;
@@ -1176,8 +1176,9 @@ std::optional<SfxItemSet> SfxBindings::CreateSet_Impl
             if ( rFound[i].nWhichId+1 != rFound[i+1].nWhichId )
                 break;
         const sal_uInt16 nWhich2 = rFound[i++].nWhichId;
-        aSet.MergeRange(nWhich1, nWhich2);
+        ranges = ranges.MergeRange(nWhich1, nWhich2);
     }
+    SfxItemSet aSet(rPool, std::move(ranges));
     return aSet;
 }
 
diff --git a/sfx2/source/dialog/tabdlg.cxx b/sfx2/source/dialog/tabdlg.cxx
index fa276f8b7400..18bfd7bd18f1 100644
--- a/sfx2/source/dialog/tabdlg.cxx
+++ b/sfx2/source/dialog/tabdlg.cxx
@@ -421,14 +421,12 @@ IMPL_LINK_NOARG(SfxTabDialogController, ResetHdl, weld::Button&, void)
         m_xExampleSet.reset(new SfxItemSet(*m_pSet));
 
     const SfxItemPool* pPool = m_pSet->GetPool();
-    const sal_uInt16* pTmpRanges = (pDataObject->fnGetRanges)();
+    const WhichRangesContainer& pTmpRanges = (pDataObject->fnGetRanges)();
 
-    while (*pTmpRanges)
+    for (const auto & rPair : pTmpRanges)
     {
-        const sal_uInt16* pU = pTmpRanges + 1;
-
         // Correct Range with multiple values
-        sal_uInt16 nTmp = *pTmpRanges, nTmpEnd = *pU;
+        sal_uInt16 nTmp = rPair.first, nTmpEnd = rPair.second;
         DBG_ASSERT(nTmp <= nTmpEnd, "Range is sorted the wrong way");
 
         if (nTmp > nTmpEnd)
@@ -454,8 +452,6 @@ IMPL_LINK_NOARG(SfxTabDialogController, ResetHdl, weld::Button&, void)
             }
             nTmp++;
         }
-        // Go to the next pair
-        pTmpRanges += 2;
     }
 }
 
@@ -479,15 +475,13 @@ IMPL_LINK_NOARG(SfxTabDialogController, BaseFmtHdl, weld::Button&, void)
         m_xExampleSet.reset(new SfxItemSet(*m_pSet));
 
     const SfxItemPool* pPool = m_pSet->GetPool();
-    const sal_uInt16* pTmpRanges = (pDataObject->fnGetRanges)();
+    const WhichRangesContainer& pTmpRanges = (pDataObject->fnGetRanges)();
     SfxItemSet aTmpSet(*m_xExampleSet);
 
-    while (*pTmpRanges)
+    for (const auto& rPair : pTmpRanges)
     {
-        const sal_uInt16* pU = pTmpRanges + 1;
-
         // Correct Range with multiple values
-        sal_uInt16 nTmp = *pTmpRanges, nTmpEnd = *pU;
+        sal_uInt16 nTmp = rPair.first, nTmpEnd = rPair.second;
         DBG_ASSERT( nTmp <= nTmpEnd, "Range is sorted the wrong way" );
 
         if ( nTmp > nTmpEnd )
@@ -507,8 +501,6 @@ IMPL_LINK_NOARG(SfxTabDialogController, BaseFmtHdl, weld::Button&, void)
             m_pOutSet->InvalidateItem(nWh);
             nTmp++;
         }
-        // Go to the next pair
-        pTmpRanges += 2;
     }
     // Set all Items as new  -> the call the current Page Reset()
     assert(pDataObject->xTabPage && "the Page is gone");
@@ -659,7 +651,7 @@ bool SfxTabDialogController::PrepareLeaveCurrentPage()
     return bEnd;
 }
 
-const sal_uInt16* SfxTabDialogController::GetInputRanges(const SfxItemPool& rPool)
+const WhichRangesContainer & SfxTabDialogController::GetInputRanges(const SfxItemPool& rPool)
 
 /*  [Description]
 
@@ -686,8 +678,8 @@ const sal_uInt16* SfxTabDialogController::GetInputRanges(const SfxItemPool& rPoo
         return m_pSet->GetRanges();
     }
 
-    if ( m_pRanges )
-        return m_pRanges.get();
+    if ( !m_pRanges.empty() )
+        return m_pRanges;
     SfxItemSet aUS(const_cast<SfxItemPool&>(rPool));
 
     for (auto const& elem : m_pImpl->aData)
@@ -695,26 +687,19 @@ const sal_uInt16* SfxTabDialogController::GetInputRanges(const SfxItemPool& rPoo
 
         if ( elem->fnGetRanges )
         {
-            const sal_uInt16* pTmpRanges = (elem->fnGetRanges)();
+            const WhichRangesContainer& pTmpRanges = (elem->fnGetRanges)();
 
-            for (const sal_uInt16* pIter = pTmpRanges; *pIter;)
+            for (const auto & rPair : pTmpRanges)
             {
-                sal_uInt16 nWidFrom = rPool.GetWhich(*pIter++);
-                assert(*pIter);
-                sal_uInt16 nWidTo = rPool.GetWhich(*pIter++);
+                sal_uInt16 nWidFrom = rPool.GetWhich(rPair.first);
+                sal_uInt16 nWidTo = rPool.GetWhich(rPair.second);
                 aUS.MergeRange(nWidFrom, nWidTo); // Keep it valid
             }
         }
     }
 
-    int nSize = 0;
-    for (const sal_uInt16* pIter = aUS.GetRanges(); pIter && *pIter; pIter++)
-        ++nSize;
-
-    m_pRanges.reset(new sal_uInt16[nSize + 1]);
-    std::copy(aUS.GetRanges(), aUS.GetRanges() + nSize, m_pRanges.get());
-    m_pRanges[nSize] = 0;
-    return m_pRanges.get();
+    m_pRanges = aUS.GetRanges();
+    return m_pRanges;
 }
 
 SfxTabDialogController::~SfxTabDialogController()
diff --git a/sfx2/source/view/viewfrm.cxx b/sfx2/source/view/viewfrm.cxx
index 95d1d5d1dfeb..1809c9c136cc 100644
--- a/sfx2/source/view/viewfrm.cxx
+++ b/sfx2/source/view/viewfrm.cxx
@@ -2538,12 +2538,12 @@ void SfxViewFrame::StateView_Impl
         // I'm just on reload and am yielding myself ...
         return;
 
-    const sal_uInt16 *pRanges = rSet.GetRanges();
-    assert(pRanges && "Set with no Range");
-    while ( *pRanges )
+    const WhichRangesContainer & pRanges = rSet.GetRanges();
+    assert(!pRanges.empty() && "Set with no Range");
+    for ( auto const & pRange : pRanges )
     {
-        sal_uInt16 nStartWhich = *pRanges++;
-        sal_uInt16 nEndWhich = *pRanges++;
+        sal_uInt16 nStartWhich = pRange.first;
+        sal_uInt16 nEndWhich = pRange.second;
         for ( sal_uInt16 nWhich = nStartWhich; nWhich <= nEndWhich; ++nWhich )
         {
             switch(nWhich)
@@ -3064,11 +3064,11 @@ void SfxViewFrame::MiscExec_Impl( SfxRequest& rReq )
 
 void SfxViewFrame::MiscState_Impl(SfxItemSet &rSet)
 {
-    const sal_uInt16 *pRanges = rSet.GetRanges();
-    DBG_ASSERT(pRanges && *pRanges, "Set without range");
-    while ( *pRanges )
+    const WhichRangesContainer & pRanges = rSet.GetRanges();
+    DBG_ASSERT(!pRanges.empty(), "Set without range");
+    for ( auto const & pRange : pRanges )
     {
-        for(sal_uInt16 nWhich = *pRanges++; nWhich <= *pRanges; ++nWhich)
+        for(sal_uInt16 nWhich = pRange.first; nWhich <= pRange.second; ++nWhich)
         {
             switch(nWhich)
             {
@@ -3166,8 +3166,6 @@ void SfxViewFrame::MiscState_Impl(SfxItemSet &rSet)
                     break;
             }
         }
-
-        ++pRanges;
     }
 }
 
diff --git a/sfx2/source/view/viewfrm2.cxx b/sfx2/source/view/viewfrm2.cxx
index 370459a3ec0f..744a8fe1bcb1 100644
--- a/sfx2/source/view/viewfrm2.cxx
+++ b/sfx2/source/view/viewfrm2.cxx
@@ -267,11 +267,11 @@ void SfxViewFrame::GetState_Impl( SfxItemSet &rSet )
     if ( !pDocSh )
         return;
 
-    const sal_uInt16 *pRanges = rSet.GetRanges();
-    DBG_ASSERT(pRanges, "Set without Range");
-    while ( *pRanges )
+    const WhichRangesContainer & pRanges = rSet.GetRanges();
+    DBG_ASSERT(!pRanges.empty(), "Set without Range");
+    for ( auto const & pRange : pRanges )
     {
-        for ( sal_uInt16 nWhich = *pRanges++; nWhich <= *pRanges; ++nWhich )
+        for ( sal_uInt16 nWhich = pRange.first; nWhich <= pRange.second; ++nWhich )
         {
             switch(nWhich)
             {
@@ -314,7 +314,6 @@ void SfxViewFrame::GetState_Impl( SfxItemSet &rSet )
                 OSL_FAIL( "invalid message-id" );
             }
         }
-        ++pRanges;
     }
 }