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>

5 files changed, 357 insertions, 302 deletions

diff --git a/svl/source/inc/items_helper.hxx b/svl/source/inc/items_helper.hxx
index 75582d9f8741..da81ecf334da 100644
--- a/svl/source/inc/items_helper.hxx
+++ b/svl/source/inc/items_helper.hxx
@@ -30,12 +30,7 @@
 
 namespace svl::detail
 {
-/**
- * Determines the number of sal_uInt16s in a 0-terminated array of pairs of
- * sal_uInt16s, each representing a range of sal_uInt16s, and total capacity of the ranges.
- * The terminating 0 is included in the count.
- */
-inline std::pair<sal_uInt16, sal_uInt16> CountRanges(const sal_uInt16* pRanges)
+inline std::pair<sal_uInt16, sal_uInt16> CountRangesOld(const sal_uInt16* pRanges)
 {
     sal_uInt16 nCount = 0;
     sal_uInt16 nCapacity = 0;
@@ -51,75 +46,32 @@ inline std::pair<sal_uInt16, sal_uInt16> CountRanges(const sal_uInt16* pRanges)
     }
     return { nCount, nCapacity };
 }
-
-// Adds a range to which ranges, keeping the ranges in valid state (sorted, non-overlapping)
-inline std::unique_ptr<sal_uInt16[]> MergeRange(const sal_uInt16* pWhichRanges, sal_uInt16 nFrom,
-                                                sal_uInt16 nTo)
+/**
+ * Determines the number of sal_uInt16s in a container of pairs of
+ * sal_uInt16s, each representing a range of sal_uInt16s, and total capacity of the ranges.
+ */
+inline sal_uInt16 CountRanges(const WhichRangesContainer& pRanges)
 {
-    assert(validRange(nFrom, nTo));
-
-    if (!pWhichRanges)
-    {
-        auto pNewRanges = std::make_unique<sal_uInt16[]>(3);
-        pNewRanges[0] = nFrom;
-        pNewRanges[1] = nTo;
-        pNewRanges[2] = 0;
-        return pNewRanges;
-    }
-
-    assert(validRanges(pWhichRanges));
-
-    // create vector of ranges (sal_uInt16 pairs of lower and upper bound)
-    const size_t nOldCount = CountRanges(pWhichRanges).first;
-    std::vector<std::pair<sal_uInt16, sal_uInt16>> aRangesTable;
-    aRangesTable.reserve(nOldCount / 2 + 1);
-    bool bAdded = false;
-    for (size_t i = 0; i + 1 < nOldCount; i += 2)
+    sal_uInt16 nCapacity = 0;
+    for (const auto& rPair : pRanges)
     {
-        if (!bAdded && pWhichRanges[i] >= nFrom)
-        { // insert new range, keep ranges sorted
-            aRangesTable.emplace_back(std::pair<sal_uInt16, sal_uInt16>(nFrom, nTo));
-            bAdded = true;
-        }
-        // insert current range
-        aRangesTable.emplace_back(
-            std::pair<sal_uInt16, sal_uInt16>(pWhichRanges[i], pWhichRanges[i + 1]));
+        nCapacity += rangeSize(rPair.first, rPair.second);
     }
-    if (!bAdded)
-        aRangesTable.emplace_back(std::pair<sal_uInt16, sal_uInt16>(nFrom, nTo));
-
-    // true if ranges overlap or adjoin, false if ranges are separate
-    auto needMerge
-        = [](std::pair<sal_uInt16, sal_uInt16> lhs, std::pair<sal_uInt16, sal_uInt16> rhs) {
-              return (lhs.first - 1) <= rhs.second && (rhs.first - 1) <= lhs.second;
-          };
+    return nCapacity;
+}
 
-    auto it = aRangesTable.begin();
-    // we got at least one range
-    for (;;)
+inline bool validRanges2(const WhichRangesContainer& pRanges)
+{
+    for (sal_Int32 i = 0; i < pRanges.size(); ++i)
     {
-        auto itNext = std::next(it);
-        if (itNext == aRangesTable.end())
-            break;
-        // check neighbouring ranges, find first range which overlaps or adjoins a previous range
-        if (needMerge(*it, *itNext))
-        {
-            // lower bounds are sorted, implies: it->first = min(it[0].first, it[1].first)
-            it->second = std::max(it->second, itNext->second);
-            aRangesTable.erase(itNext);
-        }
-        else
-            ++it;
+        auto p = pRanges[i];
+        if (!validRange(p.first, p.second))
+            return false;
+        // ranges must be sorted
+        if (i < pRanges.size() - 1 && !validGap(p.second, pRanges[i + 1].first))
+            return false;
     }
-
-    // construct range array
-    const size_t nNewSize = 2 * aRangesTable.size() + 1;
-    auto pNewRanges = std::make_unique<sal_uInt16[]>(nNewSize);
-    for (size_t i = 0; i < (nNewSize - 1); i += 2)
-        std::tie(pNewRanges[i], pNewRanges[i + 1]) = aRangesTable[i / 2];
-
-    pNewRanges[nNewSize - 1] = 0;
-    return pNewRanges;
+    return true;
 }
 }
 
diff --git a/svl/source/inc/poolio.hxx b/svl/source/inc/poolio.hxx
index 3ec1be562a01..b2819cc46bd5 100644
--- a/svl/source/inc/poolio.hxx
+++ b/svl/source/inc/poolio.hxx
@@ -157,7 +157,7 @@ struct SfxItemPool_Impl
     std::vector<SfxPoolItem*>*      mpStaticDefaults;
     SfxItemPool*                    mpMaster;
     rtl::Reference<SfxItemPool>     mpSecondary;
-    std::unique_ptr<sal_uInt16[]>   mpPoolRanges;
+    WhichRangesContainer            mpPoolRanges;
     sal_uInt16                      mnStart;
     sal_uInt16                      mnEnd;
     MapUnit                         eDefMetric;
diff --git a/svl/source/items/itempool.cxx b/svl/source/items/itempool.cxx
index f4976b1b7767..fe13cfb96a32 100644
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -765,26 +765,25 @@ SfxItemPool* SfxItemPool::GetMasterPool() const
  */
 void SfxItemPool::FreezeIdRanges()
 {
-    assert(!pImpl->mpPoolRanges && "pool already frozen, cannot freeze twice");
+    assert(pImpl->mpPoolRanges.empty() && "pool already frozen, cannot freeze twice");
     FillItemIdRanges_Impl( pImpl->mpPoolRanges );
 }
 
 
-void SfxItemPool::FillItemIdRanges_Impl( std::unique_ptr<sal_uInt16[]>& pWhichRanges ) const
+void SfxItemPool::FillItemIdRanges_Impl( WhichRangesContainer& pWhichRanges ) const
 {
-    DBG_ASSERT( !pImpl->mpPoolRanges, "GetFrozenRanges() would be faster!" );
+    DBG_ASSERT( pImpl->mpPoolRanges.empty(), "GetFrozenRanges() would be faster!" );
 
     pWhichRanges.reset();
 
     // Merge all ranges, keeping them sorted
     for (const SfxItemPool* pPool = this; pPool; pPool = pPool->pImpl->mpSecondary.get())
-        pWhichRanges = svl::detail::MergeRange(pWhichRanges.get(), pPool->pImpl->mnStart,
-                                               pPool->pImpl->mnEnd);
+        pWhichRanges = pWhichRanges.MergeRange(pPool->pImpl->mnStart, pPool->pImpl->mnEnd);
 }
 
-const sal_uInt16* SfxItemPool::GetFrozenIdRanges() const
+const WhichRangesContainer& SfxItemPool::GetFrozenIdRanges() const
 {
-    return pImpl->mpPoolRanges.get();
+    return pImpl->mpPoolRanges;
 }
 
 const SfxPoolItem *SfxItemPool::GetItem2Default(sal_uInt16 nWhich) const
diff --git a/svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx
index 5b986167686f..66673ad69968 100644
--- a/svl/source/items/itemset.cxx
+++ b/svl/source/items/itemset.cxx
@@ -47,36 +47,54 @@ SfxItemSet::SfxItemSet(SfxItemPool& rPool)
     , m_pParent(nullptr)
     , m_nCount(0)
 {
-    m_pWhichRanges = const_cast<sal_uInt16*>(m_pPool->GetFrozenIdRanges());
-    assert( m_pWhichRanges && "don't create ItemSets with full range before FreezeIdRanges()" );
-    if (!m_pWhichRanges)
-    {
-        std::unique_ptr<sal_uInt16[]> tmp;
-        m_pPool->FillItemIdRanges_Impl(tmp);
-        m_pWhichRanges = tmp.release();
-    }
-    assert(svl::detail::validRanges(m_pWhichRanges));
+    m_pWhichRanges = m_pPool->GetFrozenIdRanges();
+    assert( !m_pWhichRanges.empty() && "don't create ItemSets with full range before FreezeIdRanges()" );
+    assert(svl::detail::validRanges2(m_pWhichRanges));
 
     const sal_uInt16 nSize = TotalCount();
     m_pItems.reset(new const SfxPoolItem*[nSize]{});
 }
 
-sal_uInt16 SfxItemSet::InitRanges_Impl(const sal_uInt16 *pWhichPairTable)
+SfxItemSet::SfxItemSet(
+    SfxItemPool & pool, std::initializer_list<sal_uInt16> wids,
+    std::size_t items):
+    m_pPool(&pool), m_pParent(nullptr),
+    m_pItems(new SfxPoolItem const *[items]{}),
+        // cannot overflow, assuming std::size_t is no smaller than sal_uInt16,
+        // as wids.size() must be substantially smaller than
+        // std::numeric_limits<sal_uInt16>::max() by construction in
+        // SfxItemSet::create
+    m_nCount(0)
+{
+    assert(wids.size() != 0);
+    assert(wids.size() % 2 == 0);
+    std::unique_ptr<WhichPair[]> xPairs = std::make_unique<WhichPair[]>(wids.size()/2);
+    std::size_t i = 0;
+    for (auto it = wids.begin(); it != wids.end(); ) {
+        sal_uInt16 nStart = *it;
+        ++it;
+        sal_uInt16 nEnd = *it;
+        ++it;
+        xPairs[i++] = { nStart, nEnd };
+    }
+    m_pWhichRanges = WhichRangesContainer(std::move(xPairs), wids.size()/2);
+    assert(svl::detail::validRanges2(m_pWhichRanges));
+}
+
+SfxItemSet::SfxItemSet( SfxItemPool& rPool, SfxAllItemSetFlag )
+    : m_pPool(&rPool)
+    , m_pParent(nullptr)
+    , m_nCount(0)
 {
-    const auto& [nCnt, nCap] = svl::detail::CountRanges(pWhichPairTable);
-    m_pItems.reset(new const SfxPoolItem* [nCap] {});
-    m_pWhichRanges = new sal_uInt16[nCnt];
-    memcpy(m_pWhichRanges, pWhichPairTable, sizeof(sal_uInt16) * nCnt);
-    assert(svl::detail::validRanges(m_pWhichRanges));
-    return nCap;
 }
 
 SfxItemSet::SfxItemSet(
-    SfxItemPool & pool, std::initializer_list<sal_uInt16> wids,
+    SfxItemPool & pool,
+    const WhichRangesContainer& wids,
     std::size_t items):
     m_pPool(&pool), m_pParent(nullptr),
     m_pItems(new SfxPoolItem const *[items]{}),
-    m_pWhichRanges(new sal_uInt16[wids.size() + 1]),
+    m_pWhichRanges(wids),
         // cannot overflow, assuming std::size_t is no smaller than sal_uInt16,
         // as wids.size() must be substantially smaller than
         // std::numeric_limits<sal_uInt16>::max() by construction in
@@ -84,56 +102,90 @@ SfxItemSet::SfxItemSet(
     m_nCount(0)
 {
     assert(wids.size() != 0);
-    assert(wids.size() % 2 == 0);
-    std::copy(wids.begin(), wids.end(), m_pWhichRanges);
-    m_pWhichRanges[wids.size()] = 0;
-    assert(svl::detail::validRanges(m_pWhichRanges));
+    assert(svl::detail::validRanges2(m_pWhichRanges));
+}
+
+SfxItemSet::SfxItemSet(
+    SfxItemPool & pool,
+    WhichRangesContainer&& wids,
+    std::size_t items):
+    m_pPool(&pool), m_pParent(nullptr),
+    m_pItems(new SfxPoolItem const *[items]{}),
+    m_pWhichRanges(std::move(wids)),
+        // cannot overflow, assuming std::size_t is no smaller than sal_uInt16,
+        // as wids.size() must be substantially smaller than
+        // std::numeric_limits<sal_uInt16>::max() by construction in
+        // SfxItemSet::create
+    m_nCount(0)
+{
+    assert(m_pWhichRanges.size() != 0);
+    assert(svl::detail::validRanges2(m_pWhichRanges));
+}
+
+SfxItemSet::SfxItemSet(
+    SfxItemPool & pool, const WhichRangesContainer& wids):
+    m_pPool(&pool), m_pParent(nullptr),
+    m_pWhichRanges(wids),
+    m_nCount(0)
+{
+    assert(wids.size() != 0);
+    assert(svl::detail::validRanges2(m_pWhichRanges));
+    std::size_t size = svl::detail::CountRanges(wids);
+    m_pItems.reset( new SfxPoolItem const *[size]{} );
+}
+
+SfxItemSet::SfxItemSet(
+    SfxItemPool & pool, WhichRangesContainer&& wids):
+    m_pPool(&pool), m_pParent(nullptr),
+    m_pWhichRanges(std::move(wids)),
+    m_nCount(0)
+{
+    assert(svl::detail::validRanges2(m_pWhichRanges));
+    std::size_t size = svl::detail::CountRanges(m_pWhichRanges);
+    m_pItems.reset( new SfxPoolItem const *[size]{} );
 }
 
 SfxItemSet::SfxItemSet(
     SfxItemPool & pool, std::initializer_list<Pair> wids):
     m_pPool(&pool), m_pParent(nullptr),
-    m_pWhichRanges(new sal_uInt16[2 * wids.size() + 1]), //TODO: overflow
     m_nCount(0)
 {
     assert(wids.size() != 0);
+    std::unique_ptr<WhichPair[]> xPairs = std::make_unique<WhichPair[]>(wids.size());
     std::size_t i = 0;
     std::size_t size = 0;
     for (auto const & p: wids) {
-        m_pWhichRanges[i++] = p.wid1;
-        m_pWhichRanges[i++] = p.wid2;
+        xPairs[i++] = { p.wid1, p.wid2 };
         size += svl::detail::rangeSize(p.wid1, p.wid2);
             // cannot overflow, assuming std::size_t is no smaller than
             // sal_uInt16
     }
-    m_pWhichRanges[i] = 0;
-    assert(svl::detail::validRanges(m_pWhichRanges));
+    m_pWhichRanges = WhichRangesContainer(std::move(xPairs), wids.size());
+    assert(svl::detail::validRanges2(m_pWhichRanges));
     m_pItems.reset( new SfxPoolItem const *[size]{} );
 }
 
 SfxItemSet::SfxItemSet( SfxItemPool& rPool, const sal_uInt16* pWhichPairTable )
     : m_pPool(&rPool)
     , m_pParent(nullptr)
-    , m_pWhichRanges(nullptr)
     , m_nCount(0)
 {
-    // pWhichPairTable == 0 is for the SfxAllEnumItemSet
-    if ( pWhichPairTable )
-        InitRanges_Impl(pWhichPairTable);
+    const auto& [nCnt, nCap] = svl::detail::CountRangesOld(pWhichPairTable);
+    m_pItems.reset(new const SfxPoolItem* [nCap] {});
+    m_pWhichRanges = WhichRangesContainer(reinterpret_cast<const WhichPair*>(pWhichPairTable), (nCnt-1)/2);
 }
 
 SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
     : m_pPool( rASet.m_pPool )
     , m_pParent( rASet.m_pParent )
+    , m_pWhichRanges( rASet.m_pWhichRanges )
     , m_nCount( rASet.m_nCount )
 {
-    if (!rASet.m_pWhichRanges)
-    {
-        m_pWhichRanges = nullptr;
+    if (rASet.m_pWhichRanges.empty())
         return;
-    }
 
-    sal_uInt16 nCnt = InitRanges_Impl(rASet.m_pWhichRanges);
+    auto nCnt = svl::detail::CountRanges(m_pWhichRanges);
+    m_pItems.reset(new const SfxPoolItem* [nCnt] {});
 
     // Copy attributes
     SfxPoolItem const** ppDst = m_pItems.get();
@@ -156,26 +208,25 @@ SfxItemSet::SfxItemSet( const SfxItemSet& rASet )
             // !IsPoolable() => assign via Pool
             *ppDst = &m_pPool->Put( **ppSrc );
 
-    assert(svl::detail::validRanges(m_pWhichRanges));
+    assert(svl::detail::validRanges2(m_pWhichRanges));
 }
 
 SfxItemSet::SfxItemSet(SfxItemSet&& rASet) noexcept
     : m_pPool( rASet.m_pPool )
     , m_pParent( rASet.m_pParent )
     , m_pItems( std::move(rASet.m_pItems) )
-    , m_pWhichRanges( rASet.m_pWhichRanges )
+    , m_pWhichRanges( std::move(rASet.m_pWhichRanges) )
     , m_nCount( rASet.m_nCount )
 {
     rASet.m_pPool = nullptr;
     rASet.m_pParent = nullptr;
-    rASet.m_pWhichRanges = nullptr;
     rASet.m_nCount = 0;
-    assert(svl::detail::validRanges(m_pWhichRanges));
+    assert(svl::detail::validRanges2(m_pWhichRanges));
 }
 
 SfxItemSet::~SfxItemSet()
 {
-    if (m_pWhichRanges) // might be nullptr if we have been moved-from
+    if (!m_pWhichRanges.empty()) // might be nullptr if we have been moved-from
     {
         sal_uInt16 nCount = TotalCount();
         if( Count() )
@@ -200,9 +251,7 @@ SfxItemSet::~SfxItemSet()
     }
 
     m_pItems.reset();
-    if (m_pPool && m_pWhichRanges != m_pPool->GetFrozenIdRanges())
-        delete[] m_pWhichRanges;
-    m_pWhichRanges = nullptr; // for invariant-testing
+    m_pWhichRanges.reset(); // for invariant-testing
 }
 
 /**
@@ -218,14 +267,13 @@ sal_uInt16 SfxItemSet::ClearItem( sal_uInt16 nWhich )
 
     if( nWhich )
     {
-        const sal_uInt16* pPtr = m_pWhichRanges;
-        while( *pPtr )
+        for (const WhichPair& rPair : m_pWhichRanges)
         {
             // Within this range?
-            if( *pPtr <= nWhich && nWhich <= *(pPtr+1) )
+            if( rPair.first <= nWhich && nWhich <= rPair.second )
             {
                 // Actually set?
-                ppFnd += nWhich - *pPtr;
+                ppFnd += nWhich - rPair.first;
                 if( *ppFnd )
                 {
                     // Due to the assertions in the sub calls, we need to do the following
@@ -252,18 +300,16 @@ sal_uInt16 SfxItemSet::ClearItem( sal_uInt16 nWhich )
                 // found => break
                 break;
             }
-            ppFnd += *(pPtr+1) - *pPtr + 1;
-            pPtr += 2;
+            ppFnd += rPair.second - rPair.first + 1;
         }
     }
     else
     {
         nDel = m_nCount;
 
-        sal_uInt16* pPtr = m_pWhichRanges;
-        while( *pPtr )
+        for (const WhichPair& rPair : m_pWhichRanges)
         {
-            for( nWhich = *pPtr; nWhich <= *(pPtr+1); ++nWhich, ++ppFnd )
+            for( nWhich = rPair.first; nWhich <= rPair.second; ++nWhich, ++ppFnd )
                 if( *ppFnd )
                 {
                     // Due to the assertions in the sub calls, we need to do this
@@ -296,7 +342,6 @@ sal_uInt16 SfxItemSet::ClearItem( sal_uInt16 nWhich )
                         }
                     }
                 }
-            pPtr += 2;
         }
     }
     return nDel;
@@ -304,17 +349,15 @@ sal_uInt16 SfxItemSet::ClearItem( sal_uInt16 nWhich )
 
 void SfxItemSet::ClearInvalidItems()
 {
-    sal_uInt16* pPtr = m_pWhichRanges;
     SfxPoolItem const** ppFnd = m_pItems.get();
-    while( *pPtr )
+    for (const WhichPair& rPair : m_pWhichRanges)
     {
-        for( sal_uInt16 nWhich = *pPtr; nWhich <= *(pPtr+1); ++nWhich, ++ppFnd )
+        for( sal_uInt16 nWhich = rPair.first; nWhich <= rPair.second; ++nWhich, ++ppFnd )
             if( IsInvalidItem(*ppFnd) )
             {
                 *ppFnd = nullptr;
                 --m_nCount;
             }
-        pPtr += 2;
     }
 }
 
@@ -335,39 +378,34 @@ SfxItemState SfxItemSet::GetItemState( sal_uInt16 nWhich,
     do
     {
         SfxPoolItem const** ppFnd = pCurrentSet->m_pItems.get();
-        const sal_uInt16* pPtr = pCurrentSet->m_pWhichRanges;
-        if (pPtr)
+        for (const WhichPair& rPair : pCurrentSet->m_pWhichRanges)
         {
-            while ( *pPtr )
+            if ( rPair.first <= nWhich && nWhich <= rPair.second )
             {
-                if ( *pPtr <= nWhich && nWhich <= *(pPtr+1) )
+                // Within this range
+                ppFnd += nWhich - rPair.first;
+                if ( !*ppFnd )
                 {
-                    // Within this range
-                    ppFnd += nWhich - *pPtr;
-                    if ( !*ppFnd )
-                    {
-                        eRet = SfxItemState::DEFAULT;
-                        if( !bSrchInParent )
-                            return eRet; // Not present
-                        break; // Keep searching in the parents!
-                    }
+                    eRet = SfxItemState::DEFAULT;
+                    if( !bSrchInParent )
+                        return eRet; // Not present
+                    break; // Keep searching in the parents!
+                }
 
-                    if ( IsInvalidItem(*ppFnd) )
-                        // Different ones are present
-                        return SfxItemState::DONTCARE;
+                if ( IsInvalidItem(*ppFnd) )
+                    // Different ones are present
+                    return SfxItemState::DONTCARE;
 
-                    if ( (*ppFnd)->IsVoidItem() )
-                        return SfxItemState::DISABLED;
+                if ( (*ppFnd)->IsVoidItem() )
+                    return SfxItemState::DISABLED;
 
-                    if (ppItem)
-                    {
-                        *ppItem = *ppFnd;
-                    }
-                    return SfxItemState::SET;
+                if (ppItem)
+                {
+                    *ppItem = *ppFnd;
                 }
-                ppFnd += *(pPtr+1) - *pPtr + 1;
-                pPtr += 2;
+                return SfxItemState::SET;
             }
+            ppFnd += rPair.second - rPair.first + 1;
         }
         if (!bSrchInParent)
             break;
@@ -393,13 +431,12 @@ const SfxPoolItem* SfxItemSet::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nWh
     }
 
     SfxPoolItem const** ppFnd = m_pItems.get();
-    const sal_uInt16* pPtr = m_pWhichRanges;
-    while( *pPtr )
+    for (const WhichPair& rPair : m_pWhichRanges)
     {
-        if( *pPtr <= nWhich && nWhich <= *(pPtr+1) )
+        if( rPair.first <= nWhich && nWhich <= rPair.second )
         {
             // Within this range
-            ppFnd += nWhich - *pPtr;
+            ppFnd += nWhich - rPair.first;
             if( *ppFnd ) // Already one present
             {
                 // Same Item already present?
@@ -478,8 +515,7 @@ const SfxPoolItem* SfxItemSet::PutImpl( const SfxPoolItem& rItem, sal_uInt16 nWh
                         "svl.items", "putted Item unequal, with ID/pos " << nWhich );
             return *ppFnd;
         }
-        ppFnd += *(pPtr+1) - *pPtr + 1;
-        pPtr += 2;
+        ppFnd += rPair.second - rPair.first + 1;
     }
     if (bPassingOwnership)
         delete &rItem;
@@ -492,10 +528,9 @@ bool SfxItemSet::Put( const SfxItemSet& rSet, bool bInvalidAsDefault )
     if( rSet.Count() )
     {
         SfxPoolItem const** ppFnd = rSet.m_pItems.get();
-        const sal_uInt16* pPtr = rSet.m_pWhichRanges;
-        while ( *pPtr )
+        for (const WhichPair& rPair : rSet.m_pWhichRanges)
         {
-            for ( sal_uInt16 nWhich = *pPtr; nWhich <= *(pPtr+1); ++nWhich, ++ppFnd )
+            for ( sal_uInt16 nWhich = rPair.first; nWhich <= rPair.second; ++nWhich, ++ppFnd )
                 if( *ppFnd )
                 {
                     if ( IsInvalidItem( *ppFnd ) )
@@ -510,7 +545,6 @@ bool SfxItemSet::Put( const SfxItemSet& rSet, bool bInvalidAsDefault )
                     else
                         bRet |= nullptr != Put( **ppFnd, nWhich );
                 }
-            pPtr += 2;
         }
     }
     return bRet;
@@ -540,10 +574,9 @@ void SfxItemSet::PutExtended
 {
     // don't "optimize" with "if( rSet.Count()" because of dont-care + defaults
     SfxPoolItem const** ppFnd = rSet.m_pItems.get();
-    const sal_uInt16* pPtr = rSet.m_pWhichRanges;
-    while ( *pPtr )
+    for (const WhichPair& rPair : rSet.m_pWhichRanges)
     {
-        for ( sal_uInt16 nWhich = *pPtr; nWhich <= *(pPtr+1); ++nWhich, ++ppFnd )
+        for ( sal_uInt16 nWhich = rPair.first; nWhich <= rPair.second; ++nWhich, ++ppFnd )
             if( *ppFnd )
             {
                 if ( IsInvalidItem( *ppFnd ) )
@@ -592,7 +625,6 @@ void SfxItemSet::PutExtended
                         assert(!"invalid Argument for eDefaultAs");
                 }
             }
-        pPtr += 2;
     }
 }
 
@@ -608,34 +640,39 @@ void SfxItemSet::MergeRange( sal_uInt16 nFrom, sal_uInt16 nTo )
             eItemState == SfxItemState::DEFAULT || eItemState == SfxItemState::SET)
             return;
 
-    auto pNewRanges = svl::detail::MergeRange(m_pWhichRanges, nFrom, nTo);
-    SetRanges(pNewRanges.get());
+    auto pNewRanges = m_pWhichRanges.MergeRange(nFrom, nTo);
+    RecreateRanges_Impl(pNewRanges);
+    m_pWhichRanges = std::move(pNewRanges);
 }
 
 /**
  * Modifies the ranges of settable items. Keeps state of items which
  * are new ranges too.
  */
-void SfxItemSet::SetRanges( const sal_uInt16 *pNewRanges )
+void SfxItemSet::SetRanges( const WhichRangesContainer& pNewRanges )
 {
     // Identical Ranges?
     if (m_pWhichRanges == pNewRanges)
         return;
-    if (m_pWhichRanges)
-    {
-        const sal_uInt16* pOld = m_pWhichRanges;
-        const sal_uInt16* pNew = pNewRanges;
-        while (*pOld == *pNew)
-        {
-            if (!*pOld && !*pNew)
-                return;
-            ++pOld;
-            ++pNew;
-        }
-    }
+    assert(svl::detail::validRanges2(pNewRanges));
+    RecreateRanges_Impl(pNewRanges);
+    m_pWhichRanges = pNewRanges;
+}
 
+void SfxItemSet::SetRanges( WhichRangesContainer&& pNewRanges )
+{
+    // Identical Ranges?
+    if (m_pWhichRanges == pNewRanges)
+        return;
+    assert(svl::detail::validRanges2(pNewRanges));
+    RecreateRanges_Impl(pNewRanges);
+    m_pWhichRanges = std::move(pNewRanges);
+}
+
+void SfxItemSet::RecreateRanges_Impl(const WhichRangesContainer& pNewRanges)
+{
     // create new item-array (by iterating through all new ranges)
-    const auto& [nCount, nSize] = svl::detail::CountRanges(pNewRanges);
+    const auto nSize = svl::detail::CountRanges(pNewRanges);
     SfxPoolItem const** aNewItems = new const SfxPoolItem* [ nSize ];
     sal_uInt16 nNewCount = 0;
     if (m_nCount == 0)
@@ -643,10 +680,10 @@ void SfxItemSet::SetRanges( const sal_uInt16 *pNewRanges )
     else
     {
         sal_uInt16 n = 0;
-        for ( const sal_uInt16 *pRange = pNewRanges; *pRange; pRange += 2 )
+        for ( auto const & pRange : pNewRanges )
         {
             // iterate through all ids in the range
-            for ( sal_uInt16 nWID = *pRange; nWID <= pRange[1]; ++nWID, ++n )
+            for ( sal_uInt16 nWID = pRange.first; nWID <= pRange.second; ++nWID, ++n )
             {
                 // direct move of pointer (not via pool)
                 SfxItemState eState = GetItemState( nWID, false, aNewItems+n );
@@ -687,20 +724,6 @@ void SfxItemSet::SetRanges( const sal_uInt16 *pNewRanges )
     // replace old items-array and ranges
     m_pItems.reset( aNewItems );
     m_nCount = nNewCount;
-
-    if( pNewRanges == GetPool()->GetFrozenIdRanges() )
-    {
-        delete[] m_pWhichRanges;
-        m_pWhichRanges = const_cast<sal_uInt16*>(pNewRanges);
-    }
-    else
-    {
-        if (m_pWhichRanges != m_pPool->GetFrozenIdRanges())
-            delete[] m_pWhichRanges;
-        m_pWhichRanges = new sal_uInt16[ nCount ];
-        memcpy( m_pWhichRanges, pNewRanges, sizeof( sal_uInt16 ) * nCount );
-    }
-    assert(svl::detail::validRanges(m_pWhichRanges));
 }
 
 /**
@@ -784,13 +807,12 @@ const SfxPoolItem& SfxItemSet::Get( sal_uInt16 nWhich, bool bSrchInParent) const
         if( pCurrentSet->Count() )
         {
             SfxPoolItem const** ppFnd = pCurrentSet->m_pItems.get();
-            const sal_uInt16* pPtr = pCurrentSet->m_pWhichRanges;
-            while( *pPtr )
+            for (auto const & pPtr : pCurrentSet->m_pWhichRanges)
             {
-                if( *pPtr <= nWhich && nWhich <= *(pPtr+1) )
+                if( pPtr.first <= nWhich && nWhich <= pPtr.second )
                 {
                     // In this Range
-                    ppFnd += nWhich - *pPtr;
+                    ppFnd += nWhich - pPtr.first;
                     if( *ppFnd )
                     {
                         if( IsInvalidItem(*ppFnd) ) {
@@ -809,8 +831,7 @@ const SfxPoolItem& SfxItemSet::Get( sal_uInt16 nWhich, bool bSrchInParent) const
                     }
                     break; // Continue with Parent
                 }
-                ppFnd += *(pPtr+1) - *pPtr + 1;
-                pPtr += 2;
+                ppFnd += pPtr.second - pPtr.first + 1;
             }
         }
 //TODO: Search until end of Range: What are we supposed to do now? To the Parent or Default??
@@ -836,11 +857,9 @@ void SfxItemSet::Changed( const SfxPoolItem&, const SfxPoolItem& )
 sal_uInt16 SfxItemSet::TotalCount() const
 {
     sal_uInt16 nRet = 0;
-    sal_uInt16* pPtr = m_pWhichRanges;
-    while( *pPtr )
+    for (auto const & pPtr : m_pWhichRanges)
     {
-        nRet += ( *(pPtr+1) - *pPtr ) + 1;
-        pPtr += 2;
+        nRet += ( pPtr.second - pPtr.first ) + 1;
     }
     return nRet;
 }
@@ -862,25 +881,10 @@ void SfxItemSet::Intersect( const SfxItemSet& rSet )
         return;
     }
 
-    // Test whether the Which Ranges are different
-    sal_uInt16* pWh1 = m_pWhichRanges;
-    sal_uInt16* pWh2 = rSet.m_pWhichRanges;
-    sal_uInt16 nSize = 0;
-
-    for( sal_uInt16 n = 0; *pWh1 && *pWh2; ++pWh1, ++pWh2, ++n )
-    {
-        if( *pWh1 != *pWh2 )
-        {
-            break;
-        }
-        if( n & 1 )
-            nSize += ( *pWh1 - *(pWh1-1) ) + 1;
-    }
-    bool bEqual = *pWh1 == *pWh2; // Also check for 0
-
     // If the Ranges are identical, we can easily process it
-    if( bEqual )
+    if( m_pWhichRanges == rSet.m_pWhichRanges )
     {
+        sal_uInt16 nSize = TotalCount();
         SfxPoolItem const** ppFnd1 = m_pItems.get();
         SfxPoolItem const** ppFnd2 = rSet.m_pItems.get();
 
@@ -926,25 +930,10 @@ void SfxItemSet::Differentiate( const SfxItemSet& rSet )
     if( !Count() || !rSet.Count() )// None set?
         return;
 
-   // Test whether the Which Ranges are different
-    sal_uInt16* pWh1 = m_pWhichRanges;
-    sal_uInt16* pWh2 = rSet.m_pWhichRanges;
-    sal_uInt16 nSize = 0;
-
-    for( sal_uInt16 n = 0; *pWh1 && *pWh2; ++pWh1, ++pWh2, ++n )
-    {
-        if( *pWh1 != *pWh2 )
-        {
-            break;
-        }
-        if( n & 1 )
-            nSize += ( *pWh1 - *(pWh1-1) ) + 1;
-    }
-    bool bEqual = *pWh1 == *pWh2; // Also test for 0
-
     // If the Ranges are identical, we can easily process it
-    if( bEqual )
+    if( m_pWhichRanges == rSet.m_pWhichRanges )
     {
+        sal_uInt16 nSize = TotalCount();
         SfxPoolItem const** ppFnd1 = m_pItems.get();
         SfxPoolItem const** ppFnd2 = rSet.m_pItems.get();
 
@@ -1134,25 +1123,10 @@ void SfxItemSet::MergeValues( const SfxItemSet& rSet )
     // WARNING! When making changes/fixing bugs, always update the table above!!
     assert( GetPool() == rSet.GetPool() && "MergeValues with different Pools" );
 
-    // Test if the which Ranges are different
-    sal_uInt16* pWh1 = m_pWhichRanges;
-    sal_uInt16* pWh2 = rSet.m_pWhichRanges;
-    sal_uInt16 nSize = 0;
-
-    for( sal_uInt16 n = 0; *pWh1 && *pWh2; ++pWh1, ++pWh2, ++n )
-    {
-        if( *pWh1 != *pWh2 )
-        {
-            break;
-        }
-        if( n & 1 )
-            nSize += ( *pWh1 - *(pWh1-1) ) + 1;
-    }
-    bool bEqual = *pWh1 == *pWh2; // Also check for 0
-
     // If the Ranges match, they are easier to process!
-    if( bEqual )
+    if( m_pWhichRanges == rSet.m_pWhichRanges )
     {
+        sal_uInt16 nSize = TotalCount();
         SfxPoolItem const** ppFnd1 = m_pItems.get();
         SfxPoolItem const** ppFnd2 = rSet.m_pItems.get();
 
@@ -1184,32 +1158,29 @@ void SfxItemSet::MergeValues( const SfxItemSet& rSet )
 void SfxItemSet::MergeValue( const SfxPoolItem& rAttr, bool bIgnoreDefaults )
 {
     SfxPoolItem const** ppFnd = m_pItems.get();
-    const sal_uInt16* pPtr = m_pWhichRanges;
     const sal_uInt16 nWhich = rAttr.Which();
-    while( *pPtr )
+    for( auto const & pPtr : m_pWhichRanges )
     {
         // In this Range??
-        if( *pPtr <= nWhich && nWhich <= *(pPtr+1) )
+        if( pPtr.first <= nWhich && nWhich <= pPtr.second )
         {
-            ppFnd += nWhich - *pPtr;
+            ppFnd += nWhich - pPtr.first;
             MergeItem_Impl(m_pPool, m_nCount, ppFnd, &rAttr, bIgnoreDefaults);
             break;
         }
-        ppFnd += *(pPtr+1) - *pPtr + 1;
-        pPtr += 2;
+        ppFnd += pPtr.second - pPtr.first + 1;
     }
 }
 
 void SfxItemSet::InvalidateItem( sal_uInt16 nWhich )
 {
     SfxPoolItem const** ppFnd = m_pItems.get();
-    const sal_uInt16* pPtr = m_pWhichRanges;
-    while( *pPtr )
+    for( auto const & pPtr : m_pWhichRanges )
     {
-        if( *pPtr <= nWhich && nWhich <= *(pPtr+1) )
+        if( pPtr.first <= nWhich && nWhich <= pPtr.second )
         {
             // In this Range?
-            ppFnd += nWhich - *pPtr;
+            ppFnd += nWhich - pPtr.first;
 
             if( *ppFnd ) // Set for me
             {
@@ -1226,22 +1197,19 @@ void SfxItemSet::InvalidateItem( sal_uInt16 nWhich )
             }
             break;
         }
-        ppFnd += *(pPtr+1) - *pPtr + 1;
-        pPtr += 2;
+        ppFnd += pPtr.second - pPtr.first + 1;
     }
 }
 
 sal_uInt16 SfxItemSet::GetWhichByPos( sal_uInt16 nPos ) const
 {
     sal_uInt16 n = 0;
-    sal_uInt16* pPtr = m_pWhichRanges;
-    while( *pPtr )
+    for( auto const & pPtr : m_pWhichRanges )
     {
-        n = ( *(pPtr+1) - *pPtr ) + 1;
+        n = ( pPtr.second - pPtr.first ) + 1;
         if( nPos < n )
-            return *pPtr + nPos;
+            return pPtr.first + nPos;
         nPos = nPos - n;
-        pPtr += 2;
     }
     assert(false);
     return 0;
@@ -1270,10 +1238,9 @@ bool SfxItemSet::Equals(const SfxItemSet &rCmp, bool bComparePool) const
         return false;
 
     // Are the Ranges themselves unequal?
-    for (sal_uInt16 nRange = 0; m_pWhichRanges[nRange]; nRange += 2)
+    for (sal_Int32 i = 0; i < m_pWhichRanges.size(); ++i)
     {
-        if (m_pWhichRanges[nRange] != rCmp.m_pWhichRanges[nRange] ||
-            m_pWhichRanges[nRange+1] != rCmp.m_pWhichRanges[nRange+1])
+        if (m_pWhichRanges[i] != rCmp.m_pWhichRanges[i])
         {
             // We must use the slow method then
             SfxWhichIter aIter( *this );
@@ -1374,18 +1341,17 @@ SfxItemSet SfxItemSet::CloneAsValue(bool bItems, SfxItemPool *pToPool ) const
 void SfxItemSet::PutDirect(const SfxPoolItem &rItem)
 {
     SfxPoolItem const** ppFnd = m_pItems.get();
-    const sal_uInt16* pPtr = m_pWhichRanges;
     const sal_uInt16 nWhich = rItem.Which();
 #ifdef DBG_UTIL
     IsPoolDefaultItem(&rItem) || m_pPool->CheckItemInPool(&rItem);
         // Only cause assertion in the callees
 #endif
-    while( *pPtr )
+    for( auto const & pPtr : m_pWhichRanges)
     {
-        if( *pPtr <= nWhich && nWhich <= *(pPtr+1) )
+        if( pPtr.first <= nWhich && nWhich <= pPtr.second )
         {
             // In this Range?
-            ppFnd += nWhich - *pPtr;
+            ppFnd += nWhich - pPtr.first;
             const SfxPoolItem* pOld = *ppFnd;
             if( pOld ) // One already present
             {
@@ -1408,8 +1374,7 @@ void SfxItemSet::PutDirect(const SfxPoolItem &rItem)
 
             return;
         }
-        ppFnd += *(pPtr+1) - *pPtr + 1;
-        pPtr += 2;
+        ppFnd += pPtr.second - pPtr.first + 1;
     }
 }
 
@@ -1436,7 +1401,7 @@ void SfxItemSet::dumpAsXml(xmlTextWriterPtr pWriter) const
 // ----------------------------------------------- class SfxAllItemSet
 
 SfxAllItemSet::SfxAllItemSet( SfxItemPool &rPool )
-:   SfxItemSet(rPool, nullptr)
+:   SfxItemSet(rPool, SfxAllItemSetFlag::Flag)
 {
 }
 
@@ -1492,4 +1457,134 @@ SfxItemSet SfxAllItemSet::CloneAsValue(bool , SfxItemPool * ) const
     throw std::logic_error("cannot do this");
 }
 
+
+
+
+WhichRangesContainer::WhichRangesContainer( const WhichPair* wids, sal_Int32 nSize )
+{
+    auto p = new WhichPair[nSize];
+    memcpy(p, wids, nSize * sizeof(WhichPair));
+    m_pairs = p;
+    m_size = nSize;
+    m_bOwnRanges = true;
+}
+
+WhichRangesContainer::WhichRangesContainer(sal_uInt16 nWhichStart, sal_uInt16 nWhichEnd)
+    : m_size(1), m_bOwnRanges(true)
+{
+    auto p = new WhichPair[1];
+    p[0] = { nWhichStart, nWhichEnd };
+    m_pairs = p;
+}
+
+WhichRangesContainer::WhichRangesContainer(WhichRangesContainer && other)
+{
+    std::swap(m_pairs, other.m_pairs);
+    std::swap(m_size, other.m_size);
+    std::swap(m_bOwnRanges, other.m_bOwnRanges);
+}
+
+WhichRangesContainer& WhichRangesContainer::operator=(WhichRangesContainer && other)
+{
+    std::swap(m_pairs, other.m_pairs);
+    std::swap(m_size, other.m_size);
+    std::swap(m_bOwnRanges, other.m_bOwnRanges);
+    return *this;
+}
+
+WhichRangesContainer& WhichRangesContainer::operator=(WhichRangesContainer const & other)
+{
+    reset();
+    m_size = other.m_size;
+    m_bOwnRanges = other.m_bOwnRanges;
+    if (m_bOwnRanges)
+    {
+        auto p = new WhichPair[m_size];
+        memcpy(p, other.m_pairs, m_size * sizeof(WhichPair));
+        m_pairs = p;
+    }
+    else
+        m_pairs = other.m_pairs;
+    return *this;
+}
+
+WhichRangesContainer::~WhichRangesContainer()
+{
+    reset();
+}
+
+bool WhichRangesContainer::operator==(WhichRangesContainer const & other) const
+{
+    if (m_size != other.m_size)
+        return false;
+    if (m_pairs == other.m_pairs)
+        return true;
+    return std::equal(m_pairs, m_pairs + m_size, other.m_pairs, other.m_pairs + m_size);
+}
+
+
+void WhichRangesContainer::reset()
+{
+    if (m_bOwnRanges)
+    {
+        delete [] m_pairs;
+        m_bOwnRanges = false;
+    }
+    m_pairs = nullptr;
+    m_size = 0;
+}
+
+// Adds a range to which ranges, keeping the ranges in valid state (sorted, non-overlapping)
+WhichRangesContainer WhichRangesContainer::MergeRange(sal_uInt16 nFrom,
+                                       sal_uInt16 nTo) const
+{
+    assert(svl::detail::validRange(nFrom, nTo));
+
+    if (empty())
+        return WhichRangesContainer(nFrom, nTo);
+
+    // create vector of ranges (sal_uInt16 pairs of lower and upper bound)
+    const size_t nOldCount = size();
+    std::vector<WhichPair> aRangesTable;
+    aRangesTable.reserve(nOldCount);
+    bool bAdded = false;
+    for (const auto& rPair : *this)
+    {
+        if (!bAdded && rPair.first >= nFrom)
+        { // insert new range, keep ranges sorted
+            aRangesTable.push_back({ nFrom, nTo });
+            bAdded = true;
+        }
+        // insert current range
+        aRangesTable.emplace_back(rPair);
+    }
+    if (!bAdded)
+        aRangesTable.push_back({ nFrom, nTo });
+
+    // true if ranges overlap or adjoin, false if ranges are separate
+    auto needMerge = [](WhichPair lhs, WhichPair rhs) {
+        return (lhs.first - 1) <= rhs.second && (rhs.first - 1) <= lhs.second;
+    };
+
+    auto it = aRangesTable.begin();
+    // we got at least one range
+    for (;;)
+    {
+        auto itNext = std::next(it);
+        if (itNext == aRangesTable.end())
+            break;
+        // check neighbouring ranges, find first range which overlaps or adjoins a previous range
+        if (needMerge(*it, *itNext))
+        {
+            // lower bounds are sorted, implies: it->first = min(it[0].first, it[1].first)
+            it->second = std::max(it->second, itNext->second);
+            aRangesTable.erase(itNext);
+        }
+        else
+            ++it;
+    }
+
+    return WhichRangesContainer(aRangesTable.data(), aRangesTable.size());
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/svl/source/items/whiter.cxx b/svl/source/items/whiter.cxx
index 88b487b02d30..59f2790a530c 100644
--- a/svl/source/items/whiter.cxx
+++ b/svl/source/items/whiter.cxx
@@ -23,31 +23,40 @@
 
 SfxWhichIter::SfxWhichIter( const SfxItemSet& rSet ):
     pStart(rSet.GetRanges()),
-    pRanges(pStart),
+    pRanges(pStart.begin()),
     nOffset(0)
 {
 }
 
+sal_uInt16 SfxWhichIter::GetCurWhich() const
+{
+    if ( pRanges >= (pStart.begin() + pStart.size()) )
+        return 0;
+    return pRanges->first + nOffset;
+}
+
 sal_uInt16 SfxWhichIter::NextWhich()
 {
-    if ( 0 == pRanges[0] )
+    if ( pRanges >= (pStart.begin() + pStart.size()) )
         return 0;
 
-    const sal_uInt16 nLastWhich = pRanges[0] + nOffset;
+    const sal_uInt16 nLastWhich = pRanges->first + nOffset;
     ++nOffset;
-    if (pRanges[1] == nLastWhich)
+    if (pRanges->second == nLastWhich)
     {
-        pRanges += 2;
+        ++pRanges;
         nOffset = 0;
     }
-    return pRanges[0] + nOffset;
+    if ( pRanges >= (pStart.begin() + pStart.size()) )
+        return 0;
+    return pRanges->first + nOffset;
 }
 
 sal_uInt16 SfxWhichIter::FirstWhich()
 {
-    pRanges = pStart;
+    pRanges = pStart.begin();
     nOffset = 0;
-    return pRanges[0];
+    return pRanges->first;
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */