sw/inc/IDocumentMarkAccess.hxx | 12 +++-- sw/source/core/doc/docbm.cxx | 73 ++++++++++++++++++++++-------------- sw/source/core/inc/MarkManager.hxx | 13 ++---- sw/source/filter/basflt/shellio.cxx | 5 -- 4 files changed, 59 insertions(+), 44 deletions(-)
New commits: commit 2b9582f8e1a10e99827e692a9dab50d064ce3472 Author: Mike Kaganski <[email protected]> AuthorDate: Sat May 17 17:39:12 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Sat May 17 18:45:47 2025 +0200 Avoid storing data for delayed deduplication in MarkManager itself This decreases the size of MarkManager. The two maps (introduced in commit 8015a6c9e6b2e67184fdea6c897ff8036a488ca5) take space, and are only used when loading. Change-Id: Ic88438266e383091d0a70658073d7adbee0256ae Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185444 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx index 1575232c5eab..dcaf9258b214 100644 --- a/sw/inc/IDocumentMarkAccess.hxx +++ b/sw/inc/IDocumentMarkAccess.hxx @@ -104,11 +104,15 @@ class IDocumentMarkAccess with thousands of bookmarks. When the check is disabled using disableUniqueNameChecks, duplicate names are allowed. - When the check is eventually enabled using enableUniqueNameChecks, one pass over all - marks is performed, and all duplicated names are made unique. + When the check is eventually enabled (when the returned guard is destroyed), one pass + over all new marks is performed, and all duplicated names are made unique. */ - virtual void disableUniqueNameChecks() = 0; - virtual void enableUniqueNameChecks() = 0; + class UniqueNameChecksGuard + { + public: + virtual ~UniqueNameChecksGuard() = default; + }; + virtual std::unique_ptr<UniqueNameChecksGuard> disableUniqueNameChecks() = 0; /** Returns a mark in the document for a paragraph. If there is none, a mark will be created. diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx index 11ebc1965883..5166bf389707 100644 --- a/sw/source/core/doc/docbm.cxx +++ b/sw/source/core/doc/docbm.cxx @@ -502,36 +502,54 @@ namespace sw::mark // In the mode where the names are not checked, we need to avoid a case where there was a // bookmark, and a file is inserted at an earlier point, with the same-name bookmark, causing - // a rename of the pre-existing bookmark. m_aUsedNames and m_vUncheckedNameMarks are used for - // that. m_aUsedNames is pre-populated with the existing names (at the moment when the mode - // started); and m_vUncheckedNameMarks stores only the marks needing the checks. + // a rename of the pre-existing bookmark. usedNames and uncheckedNameMarks are used for that. + // usedNames is pre-populated with the existing names (at the moment when the mode started); + // and uncheckedNameMarks stores only the marks needing the checks. - void MarkManager::disableUniqueNameChecks() + class UniqueNameChecksGuard_impl : public MarkManager::UniqueNameChecksGuard { - if (!m_bCheckUniqueNames) - return; // nested call - m_bCheckUniqueNames = false; - assert(m_aUsedNames.empty()); + public: + UniqueNameChecksGuard_impl(MarkManager& aParent) + : parent(aParent) + { + assert(parent.m_pUniqueNameChecksGuard == nullptr); + parent.m_pUniqueNameChecksGuard = this; - // Populate the pre-existing already deduplicated names - for (auto& pMark : m_vAllMarks) - m_aUsedNames.insert(pMark->GetName().toString()); - } - void MarkManager::enableUniqueNameChecks() - { - if (m_bCheckUniqueNames) - return; - // Make sure that all previously unchecked names are unique - for (auto& pMark : m_vAllMarks) + // Populate the pre-existing already deduplicated names + for (auto& pMark : parent.m_vAllMarks) + usedNames.insert(pMark->GetName().toString()); + } + ~UniqueNameChecksGuard_impl() override { - if (!m_vUncheckedNameMarks.contains(pMark)) - continue; - pMark->SetName(getUniqueMarkName(pMark->GetName(), [this](const SwMarkName& n) - { return m_aUsedNames.insert(n.toString()).second; })); + assert(parent.m_pUniqueNameChecksGuard == this); + for (auto& pMark : parent.m_vAllMarks) + { + if (!uncheckedNameMarks.contains(pMark)) + continue; + pMark->SetName( + parent.getUniqueMarkName(pMark->GetName(), [this](const SwMarkName& n) + { return usedNames.insert(n.toString()).second; })); + } + parent.m_pUniqueNameChecksGuard = nullptr; } - m_aUsedNames.clear(); - m_vUncheckedNameMarks.clear(); - m_bCheckUniqueNames = true; + + void add(sw::mark::MarkBase* mark) { uncheckedNameMarks.insert(mark); } + + private: + MarkManager& parent; + // marks with possibly duplicating names + std::unordered_set<sw::mark::MarkBase*> uncheckedNameMarks; + // container for deduplicating names + std::unordered_set<OUString> usedNames; + }; + + std::unique_ptr<MarkManager::UniqueNameChecksGuard> MarkManager::disableUniqueNameChecks() + { + if (m_pUniqueNameChecksGuard) + return {}; // nested call + + return std::unique_ptr<MarkManager::UniqueNameChecksGuard>( + new UniqueNameChecksGuard_impl(*this)); } ::sw::mark::MarkBase* MarkManager::makeMark(const SwPaM& rPaM, @@ -652,7 +670,7 @@ namespace sw::mark // for performance reasons, we trust UnoMarks to have a (generated) unique name if (eType != IDocumentMarkAccess::MarkType::UNO_BOOKMARK) { - if (m_bCheckUniqueNames) + if (!m_pUniqueNameChecksGuard) { pMark->SetName(getUniqueMarkName( pMark->GetName(), [this](const SwMarkName& n) @@ -660,7 +678,7 @@ namespace sw::mark } else { - m_vUncheckedNameMarks.insert(pMark.get()); + m_pUniqueNameChecksGuard->add(pMark.get()); } } @@ -1370,7 +1388,6 @@ namespace sw::mark m_vFieldmarks.clear(); m_vBookmarks.clear(); m_vAnnotationMarks.clear(); - m_vUncheckedNameMarks.clear(); for (const auto & p : m_vAllMarks) delete p; m_vAllMarks.clear(); diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx index 729958192a02..bb03d6372ebe 100644 --- a/sw/source/core/inc/MarkManager.hxx +++ b/sw/source/core/inc/MarkManager.hxx @@ -32,6 +32,8 @@ namespace sw::mark { class AnnotationMark; + class UniqueNameChecksGuard_impl; + class MarkManager final : virtual public IDocumentMarkAccess { @@ -57,8 +59,7 @@ namespace sw::mark { const SwPaM& rPaM, const SwMarkName& rName ) override; - virtual void disableUniqueNameChecks() override; - virtual void enableUniqueNameChecks() override; + virtual std::unique_ptr<UniqueNameChecksGuard> disableUniqueNameChecks() override; virtual void repositionMark(::sw::mark::MarkBase* io_pMark, const SwPaM& rPaM) override; virtual bool renameMark(::sw::mark::MarkBase* io_pMark, const SwMarkName& rNewName) override; @@ -153,11 +154,6 @@ namespace sw::mark { // container for all marks, this container owns the objects it points to container_t m_vAllMarks; - // container for all marks with possibly duplicating names (m_bCheckUniqueNames mode) - std::unordered_set<sw::mark::MarkBase*> m_vUncheckedNameMarks; - // container for deduplicating names (m_bCheckUniqueNames mode) - std::unordered_set<OUString> m_aUsedNames; - // additional container for bookmarks std::vector<sw::mark::Bookmark*> m_vBookmarks; // additional container for fieldmarks @@ -172,7 +168,8 @@ namespace sw::mark { sw::mark::FieldmarkWithDropDownButton* m_pLastActiveFieldmark; - bool m_bCheckUniqueNames = true; + friend class UniqueNameChecksGuard_impl; + UniqueNameChecksGuard_impl* m_pUniqueNameChecksGuard = nullptr; }; } diff --git a/sw/source/filter/basflt/shellio.cxx b/sw/source/filter/basflt/shellio.cxx index a71d6ba48d55..38440d48eea3 100644 --- a/sw/source/filter/basflt/shellio.cxx +++ b/sw/source/filter/basflt/shellio.cxx @@ -18,7 +18,6 @@ */ #include <hintids.hxx> -#include <comphelper/scopeguard.hxx> #include <osl/diagnose.h> #include <tools/date.hxx> #include <tools/time.hxx> @@ -204,9 +203,7 @@ ErrCodeMsg SwReader::Read( const Reader& rOptions ) { // Performance mode: import all bookmarks names as defined in the document - mxDoc->getIDocumentMarkAccess()->disableUniqueNameChecks(); - comphelper::ScopeGuard perfModeGuard( - [this]() { mxDoc->getIDocumentMarkAccess()->enableUniqueNameChecks(); }); + auto perfModeGuard = mxDoc->getIDocumentMarkAccess()->disableUniqueNameChecks(); nError = po->Read(*mxDoc, msBaseURL, *pPam, maFileName); }
