From ea0f6eaa9e744915bc2824f4062027b7cd811ba1 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 4 May 2026 17:17:38 +0200 Subject: [PATCH] Reduce heap fragmentation --- .../ChapterXPathIndexerInternal.cpp | 80 ++++++++++------- .../ChapterXPathIndexerInternal.h | 5 ++ lib/KOReaderSync/ChapterXPathIndexerState.h | 90 ++++++++++++++++--- 3 files changed, 128 insertions(+), 47 deletions(-) diff --git a/lib/KOReaderSync/ChapterXPathIndexerInternal.cpp b/lib/KOReaderSync/ChapterXPathIndexerInternal.cpp index 557fc9db..ef295a8b 100644 --- a/lib/KOReaderSync/ChapterXPathIndexerInternal.cpp +++ b/lib/KOReaderSync/ChapterXPathIndexerInternal.cpp @@ -169,49 +169,55 @@ size_t visibleBytesBeforeCodepoint(const XML_Char* text, const int len, const si return visibleBytes; } -std::string normalizeXPath(const std::string& input) { +// Thread-local-free scratch reused across normalizeXPath() invocations so the +// two-phase rewrite (lowercase/strip pass → bare-element-predicate pass) costs +// at most one growing std::string per process lifetime instead of two per call. +// Single-threaded on ESP32, so a function-local static is safe. +void normalizeXPath(const std::string& input, std::string& out) { + out.clear(); if (input.empty()) { - return ""; + return; } - std::string out; - out.reserve(input.size()); + static std::string firstPass; + firstPass.clear(); + firstPass.reserve(input.size()); for (const char c : input) { const unsigned char uc = static_cast(c); if (std::isspace(uc)) { continue; } - out.push_back(static_cast(std::tolower(uc))); + firstPass.push_back(static_cast(std::tolower(uc))); } const std::string textTag = "/text()"; - const size_t textPos = out.rfind(textTag); + const size_t textPos = firstPass.rfind(textTag); if (textPos != std::string::npos) { const size_t afterText = textPos + textTag.size(); - if (afterText == out.size() || out[afterText] == '.' || out[afterText] == '[') { - out.erase(textPos); + if (afterText == firstPass.size() || firstPass[afterText] == '.' || firstPass[afterText] == '[') { + firstPass.erase(textPos); } } - const size_t lastSlash = out.rfind('/'); + const size_t lastSlash = firstPass.rfind('/'); if (lastSlash != std::string::npos) { - const size_t dotPos = out.find('.', lastSlash + 1); - if (dotPos != std::string::npos && dotPos + 1 < out.size()) { + const size_t dotPos = firstPass.find('.', lastSlash + 1); + if (dotPos != std::string::npos && dotPos + 1 < firstPass.size()) { bool allDigits = true; - for (size_t i = dotPos + 1; i < out.size(); i++) { - if (!std::isdigit(static_cast(out[i]))) { + for (size_t i = dotPos + 1; i < firstPass.size(); i++) { + if (!std::isdigit(static_cast(firstPass[i]))) { allDigits = false; break; } } if (allDigits) { - out.erase(dotPos); + firstPass.erase(dotPos); } } } - while (!out.empty() && out.back() == '/') { - out.pop_back(); + while (!firstPass.empty() && firstPass.back() == '/') { + firstPass.pop_back(); } // KOReader sometimes omits the [1] predicate for elements that are the sole @@ -219,41 +225,44 @@ std::string normalizeXPath(const std::string& input) { // In XPath, an unqualified name is equivalent to name[1] when there is only // one sibling of that type, but our parser always generates explicit indices. // Insert [1] for any bare element path segment so comparisons match. - std::string normalized; - normalized.reserve(out.size() + 16); + out.reserve(firstPass.size() + 16); size_t i = 0; - while (i < out.size()) { - if (out[i] == '/') { - normalized.push_back('/'); + while (i < firstPass.size()) { + if (firstPass[i] == '/') { + out.push_back('/'); i++; // Copy element name (letters, digits, hyphens, underscores, dots) const size_t nameStart = i; - while (i < out.size() && out[i] != '/' && out[i] != '[') { + while (i < firstPass.size() && firstPass[i] != '/' && firstPass[i] != '[') { i++; } - normalized.append(out, nameStart, i - nameStart); - if (i < out.size() && out[i] == '[') { + out.append(firstPass, nameStart, i - nameStart); + if (i < firstPass.size() && firstPass[i] == '[') { // Already has a predicate – copy it verbatim - while (i < out.size() && out[i] != ']') { - normalized.push_back(out[i++]); + while (i < firstPass.size() && firstPass[i] != ']') { + out.push_back(firstPass[i++]); } - if (i < out.size()) { - normalized.push_back(out[i++]); // ']' + if (i < firstPass.size()) { + out.push_back(firstPass[i++]); // ']' } } else if (i - nameStart > 0) { // Bare element name – insert implicit [1] - normalized.append("[1]"); + out.append("[1]"); } } else { - normalized.push_back(out[i++]); + out.push_back(firstPass[i++]); } } - - return normalized; } -std::string removeIndices(const std::string& xpath) { +std::string normalizeXPath(const std::string& input) { std::string out; + normalizeXPath(input, out); + return out; +} + +void removeIndices(const std::string& xpath, std::string& out) { + out.clear(); out.reserve(xpath.size()); bool inBracket = false; for (const char c : xpath) { @@ -269,6 +278,11 @@ std::string removeIndices(const std::string& xpath) { out.push_back(c); } } +} + +std::string removeIndices(const std::string& xpath) { + std::string out; + removeIndices(xpath, out); return out; } diff --git a/lib/KOReaderSync/ChapterXPathIndexerInternal.h b/lib/KOReaderSync/ChapterXPathIndexerInternal.h index 806c0d64..3420f9e1 100644 --- a/lib/KOReaderSync/ChapterXPathIndexerInternal.h +++ b/lib/KOReaderSync/ChapterXPathIndexerInternal.h @@ -20,6 +20,11 @@ size_t visibleBytesBeforeCodepoint(const XML_Char* text, int len, size_t targetC std::string normalizeXPath(const std::string& input); std::string removeIndices(const std::string& xpath); +// Out-parameter forms reuse the caller's string capacity instead of returning +// a new allocation per call. Use these in hot per-element loops where the same +// scratch string is repopulated thousands of times. +void normalizeXPath(const std::string& input, std::string& out); +void removeIndices(const std::string& xpath, std::string& out); int pathDepth(const std::string& xpath); bool isAncestorPath(const std::string& prefix, const std::string& path); diff --git a/lib/KOReaderSync/ChapterXPathIndexerState.h b/lib/KOReaderSync/ChapterXPathIndexerState.h index 92e0273a..06c164f9 100644 --- a/lib/KOReaderSync/ChapterXPathIndexerState.h +++ b/lib/KOReaderSync/ChapterXPathIndexerState.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -23,20 +24,38 @@ struct StackState { int skipDepth = -1; size_t totalTextBytes = 0; std::vector stack; + // Sibling-name → count map per parent depth. Index `d` holds the counts for + // children that live at depth `d` in the DOM (i.e. queried just before + // pushing a new node). Entries are cleared lazily on push rather than popped + // and reallocated, so the per-element heap churn stays bounded. std::vector> siblingCounters; - StackState() { siblingCounters.emplace_back(); } + StackState() { + // Pre-size for typical EPUB chapter nesting (well below 32 levels). Avoids + // per-element vector growth that would otherwise interleave with map node + // allocations. + stack.reserve(32); + siblingCounters.resize(32); + } void pushElement(const XML_Char* rawName) { - std::string name = toLowerStr(rawName ? rawName : ""); const size_t depth = stack.size(); if (siblingCounters.size() <= depth) { siblingCounters.resize(depth + 1); } - const int sibIdx = ++siblingCounters[depth][name]; - stack.push_back({name, sibIdx, false}); - siblingCounters.emplace_back(); - if (skipDepth < 0 && isSkippableTag(name)) { + // Lowercase the tag in place into the StackNode's own storage — the prior + // implementation called toLowerStr() which returned a fresh std::string + // per element, a major fragmentation source. Lookup into the parent's + // sibling counter map then uses the stable in-place string with no extra + // allocation. + StackNode& node = stack.emplace_back(); + node.tag.assign(rawName ? rawName : ""); + for (char& c : node.tag) { + c = static_cast(std::tolower(static_cast(c))); + } + const int sibIdx = ++siblingCounters[depth][node.tag]; + node.index = sibIdx; + if (skipDepth < 0 && isSkippableTag(node.tag)) { skipDepth = static_cast(stack.size()) - 1; } } @@ -48,10 +67,15 @@ struct StackState { if (skipDepth == static_cast(stack.size()) - 1) { skipDepth = -1; } - stack.pop_back(); - if (!siblingCounters.empty()) { - siblingCounters.pop_back(); + // Clear the just-departed element's child-counter slot in place rather + // than freeing the map: the next sibling at this depth needs an empty map + // either way, and reusing the existing buckets avoids per-pop allocator + // churn. We don't shrink siblingCounters for the same reason. + const size_t childDepth = stack.size(); + if (childDepth < siblingCounters.size()) { + siblingCounters[childDepth].clear(); } + stack.pop_back(); } void onCharData(const XML_Char*, int) {} @@ -67,19 +91,57 @@ struct StackState { bool insideBody() const { return bodyIdx() >= 0; } - std::string currentXPath(const int spineIndex) const { + // Out-parameter form: appends the path into `out` without freeing it first + // so the caller controls when to reuse vs reset capacity. Use this in hot + // paths to amortise the underlying allocation. + void buildCurrentXPath(const int spineIndex, std::string& out) const { + out.clear(); + out.append("/body/DocFragment["); + appendInt(out, spineIndex + 1); + out.append("]/body"); const int bi = bodyIdx(); - std::string xpath = "/body/DocFragment[" + std::to_string(spineIndex + 1) + "]/body"; if (bi < 0) { - return xpath; + return; } for (size_t i = static_cast(bi + 1); i < stack.size(); i++) { - xpath += "/" + stack[i].tag + "[" + std::to_string(stack[i].index) + "]"; + out.push_back('/'); + out.append(stack[i].tag); + out.push_back('['); + appendInt(out, stack[i].index); + out.push_back(']'); } - return xpath; + } + + std::string currentXPath(const int spineIndex) const { + std::string out; + buildCurrentXPath(spineIndex, out); + return out; } bool shouldSkipText(const int len) const { return skipDepth >= 0 || len <= 0 || !insideBody(); } + + private: + // Appends a non-negative int as decimal digits without allocating a temp + // std::string (std::to_string would allocate per call). + static void appendInt(std::string& out, int value) { + if (value < 0) { + out.push_back('-'); + value = -value; + } + char buf[12]; + int len = 0; + if (value == 0) { + buf[len++] = '0'; + } else { + while (value > 0) { + buf[len++] = static_cast('0' + (value % 10)); + value /= 10; + } + } + while (len-- > 0) { + out.push_back(buf[len]); + } + } }; template