Reduce heap fragmentation

2026-05-04 17:17:38 +02:00
parent 42a9c09e4c
commit ea0f6eaa9e
3 changed files with 128 additions and 47 deletions
@@ -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<unsigned char>(c);
    if (std::isspace(uc)) {
      continue;
    }
-    out.push_back(static_cast<char>(std::tolower(uc)));
+    firstPass.push_back(static_cast<char>(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<unsigned char>(out[i]))) {
+      for (size_t i = dotPos + 1; i < firstPass.size(); i++) {
+        if (!std::isdigit(static_cast<unsigned char>(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;
 }

@@ -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);

@@ -1,5 +1,6 @@
 #pragma once

+#include <cctype>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -23,20 +24,38 @@ struct StackState {
  int skipDepth = -1;
  size_t totalTextBytes = 0;
  std::vector<StackNode> 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<std::unordered_map<std::string, int>> 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<char>(std::tolower(static_cast<unsigned char>(c)));
+    }
+    const int sibIdx = ++siblingCounters[depth][node.tag];
+    node.index = sibIdx;
+    if (skipDepth < 0 && isSkippableTag(node.tag)) {
      skipDepth = static_cast<int>(stack.size()) - 1;
    }
  }
@@ -48,10 +67,15 @@ struct StackState {
    if (skipDepth == static_cast<int>(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<size_t>(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<char>('0' + (value % 10));
+        value /= 10;
+      }
+    }
+    while (len-- > 0) {
+      out.push_back(buf[len]);
+    }
+  }
 };

 template <typename StateT>