perf: Improve large CSS files handling by jdk2pq · Pull Request #779 · crosspoint-reader/crosspoint-reader

jdk2pq · 2026-02-08T22:35:47Z

Summary

Closes #766. Thank you for the help @bramschulting!

What is the goal of this PR?

First and foremost, fix issue Crash when opening specific book #766.
Through working on that, I realized the current CSS parsing/loading code can be improved dramatically for large files and still had additional performance improvements to be made, even with EPUBs with small CSS.

What changes are included?

Stream CSS parsing and reuse normalization buffers to cut allocations
Add rule limits and selector validation to release rules and free up memory when needed
Skip CSS parsing/loading entirely when "Book's Embedded Style" is off

Additional Context

My test EPUB has been updated here to include a very large CSS file to test this out

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? YES, Codex

- Stream CSS parsing and reuse normalization buffers to cut allocations - Add rule limits and selector validation to release rules and free up memory when needed - Skip CSS parsing/loading entirely when "Book's Embedded Style" is off

lib/Epub/Epub/css/CssParser.cpp

bramschulting · 2026-02-09T16:53:22Z

Nice work! This fixed #766 for me 👍

jonasdiemer · 2026-02-10T16:32:07Z

This one seems to also fix #812 (at least for "The Lamb Will Slaughter the Lion" by Margaret Killjoy).

jonasdiemer

Tried with several books (with clearing cache to reload CSS) - works like a charm. Min Free: stays at > 95kB.

There are some comments on code by @ngxson above (I did NOT perform a full code review), but none of them would be blocking IMO. So I've approved, because this PR fixes several bugs.

jdk2pq · 2026-02-10T17:04:41Z

Thanks all! I'm planning to address the review feedback tonight (US Eastern time)

* master: (25 commits) fix: Reduce MIN_SIZE_FOR_POPUP to 10KB (crosspoint-reader#809) docs: Update USER_GUIDE.md (crosspoint-reader#817) fix: Prevent sleeping when in OPDS browser / downloading books (crosspoint-reader#818) feat: Extend python debugging monitor functionality (keyword filter / suppress) (crosspoint-reader#810) docs: Update USER_GUIDE.md (crosspoint-reader#808) feat: Connect to last wifi by default (crosspoint-reader#752) feat: use natural sort in file browser (crosspoint-reader#722) fix: issue if book href are absolute url and not relative to server (crosspoint-reader#741) feat: unify navigation handling with system-wide continuous navigation (crosspoint-reader#600) feat: Add Italian hyphenation support (crosspoint-reader#584) feat: Add percentage support to CSS properties (crosspoint-reader#738) Use GITHUB_REF_NAME over GITHUB_HEAD_REF in release candidate workflow Move release candidate workflow to manual dispatch fix: Allow OTA update from RC build to full release (crosspoint-reader#778) refactor: Rename "Embedded Style" to "Book's Embedded Style" (crosspoint-reader#746) perf: optimize drawPixel() (crosspoint-reader#748) feat: wakeup target detection (crosspoint-reader#731) fix: Scrolling page items calculation (crosspoint-reader#716) feat: optimize fillRectDither (crosspoint-reader#737) fix: increase lyra sideButtonHintsWidth to 30 (crosspoint-reader#727) ...

jdk2pq · 2026-02-11T05:53:06Z

Unfortunately, I had a little less time than expected to get to this, but planning on revisiting tomorrow.

* master: fix: chore: make all debug messages uniform (crosspoint-reader#825) fix: Show "Back" in file browser if not in root, "Home" otherwise. (crosspoint-reader#822) fix: Manually trigger GPIO update in File Browser mode (crosspoint-reader#819)

* master: feat: Allow screenshot retrieval from device (crosspoint-reader#820) feat: more power saving on idle (crosspoint-reader#801)

…lly used - First, I found a bug in my code that was preventing large CSS from ever actually being read, which created false positives in loading books with large CSS unfortunately (because the books worked, but no CSS was being read). This is now fixed, and EPUBs with large CSS files should also load correctly. - Moved CssParser-caching-specific code out of Epub.cpp and into CssParser.cpp - Reverted buffered write code, since SdMan handles this already - Removed max CSS file size, since it wasn't necessary - Updated platform.ini to use correct C++ flags (and unflag) to be able to use string_view

jdk2pq · 2026-02-13T04:59:03Z

Finally ready for a re-review here!

Copilot

Pull request overview

This pull request addresses issue #766 where opening books with very large CSS files caused device crashes. The changes implement streaming CSS parsing with memory management safeguards, including rule limits, selector validation, and the ability to skip CSS loading entirely when the embedded style setting is disabled.

Changes:

Refactored CSS parser from buffered to streaming architecture with stack-allocated buffers to reduce heap allocations
Added protective limits (MAX_RULES, MAX_SELECTOR_LENGTH, MIN_FREE_HEAP_FOR_CSS) to prevent memory exhaustion
Modified cache management to be handled internally by CssParser with per-section loading/clearing
Updated build configuration to use GNU C++20 standard for modern C++ features like std::string_view

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
platformio.ini	Changed C++ standard from c++2a to gnu++2a and removed conflicting gnu++11 flag
src/activities/reader/ReaderActivity.cpp	Added logic to skip CSS loading when embedded style setting is disabled
src/RecentBooksStore.cpp	Updated epub.load() call to use new skipLoadingCss parameter
lib/Epub/Epub.h	Refactored CSS cache methods into CssParser and changed getCssParser() to non-const
lib/Epub/Epub.cpp	Removed cache management logic (now in CssParser) and added cache clearing after save
lib/Epub/Epub/css/CssParser.h	Added cachePath member, updated method signatures for internal file handling
lib/Epub/Epub/css/CssParser.cpp	Complete rewrite of parser with streaming architecture, memory limits, and stack buffers
lib/Epub/Epub/Section.cpp	Added per-section cache loading and clearing of CSS parser after use

Comments suppressed due to low confidence (2)

lib/Epub/Epub/css/CssParser.cpp:753

The bitwise shift operations lack parentheses around the shift expression, which could lead to incorrect operator precedence. While the current code works correctly due to C++ operator precedence (& has lower precedence than <<), adding parentheses would improve code clarity and prevent potential issues if the code is modified in the future. For example, (definedBits & (1 << 0)) is clearer than (definedBits & 1 << 0).

    style.defined.textAlign = (definedBits & 1 << 0) != 0;
    style.defined.fontStyle = (definedBits & 1 << 1) != 0;
    style.defined.fontWeight = (definedBits & 1 << 2) != 0;
    style.defined.textDecoration = (definedBits & 1 << 3) != 0;
    style.defined.textIndent = (definedBits & 1 << 4) != 0;
    style.defined.marginTop = (definedBits & 1 << 5) != 0;
    style.defined.marginBottom = (definedBits & 1 << 6) != 0;
    style.defined.marginLeft = (definedBits & 1 << 7) != 0;
    style.defined.marginRight = (definedBits & 1 << 8) != 0;
    style.defined.paddingTop = (definedBits & 1 << 9) != 0;
    style.defined.paddingBottom = (definedBits & 1 << 10) != 0;
    style.defined.paddingLeft = (definedBits & 1 << 11) != 0;
    style.defined.paddingRight = (definedBits & 1 << 12) != 0;

lib/Epub/Epub/css/CssParser.cpp:360

The processRuleBlockWithStyle function is called even when style.defined.anySet() is false (i.e., no properties were parsed). This wastes cycles processing empty styles. The original implementation checked if (!style.defined.anySet()) return; before processing. This check should be added back at the beginning of processRuleBlockWithStyle or in the caller before calling this function.

void CssParser::processRuleBlockWithStyle(const std::string& selectorGroup, const CssStyle& style) {
  // Check if we've reached the rule limit before processing
  if (rulesBySelector_.size() >= MAX_RULES) {
    Serial.printf("[%lu] [CSS] Reached max rules limit (%zu), stopping CSS parsing\n", millis(), MAX_RULES);
    return;
  }

  // Handle comma-separated selectors
  const auto selectors = splitOnChar(selectorGroup, ',');

  for (const auto& sel : selectors) {
    // Validate selector length before processing
    if (sel.size() > MAX_SELECTOR_LENGTH) {
      Serial.printf("[%lu] [CSS] Selector too long (%zu > %zu), skipping\n", millis(), sel.size(), MAX_SELECTOR_LENGTH);
      continue;
    }

    // Normalize the selector
    std::string key = normalized(sel);
    if (key.empty()) continue;

    // Skip if this would exceed the rule limit
    if (rulesBySelector_.size() >= MAX_RULES) {
      Serial.printf("[%lu] [CSS] Reached max rules limit, stopping selector processing\n", millis());
      return;
    }

    // Store or merge with existing
    auto it = rulesBySelector_.find(key);
    if (it != rulesBySelector_.end()) {
      it->second.applyOver(style);
    } else {
      rulesBySelector_[key] = style;
    }
  }
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-13T05:04:59Z

lib/Epub/Epub/css/CssParser.cpp

+  void push_back(char c) {
+    if (len < CAPACITY - 1) {
+      data[len++] = c;
    }
-    ++pos;
  }


The StackBuffer silently truncates content when capacity is exceeded without any indication of truncation. This can lead to malformed CSS rules being processed. The push_back method should either signal an error condition or the caller should check the buffer's capacity before processing. At minimum, when the buffer is full, the parsing should set a flag to skip the current rule.

Copilot · 2026-02-13T05:05:00Z

lib/Epub/Epub/css/CssParser.cpp

+struct StackBuffer {
+  static constexpr size_t CAPACITY = 1024;
+  char data[CAPACITY];
+  size_t len = 0;


The StackBuffer struct allocates 1024 bytes on the stack per instance. Three StackBuffer instances are created in loadFromStream (selector, declBuffer, plus the lambda captures them by reference). While they're reused, this adds significant stack usage. On embedded systems with limited stack space, this could cause stack overflow issues, especially if loadFromStream is called from a deep call stack. Consider reducing CAPACITY or using heap allocation with fixed-size buffers that are cleared/reused.

I only see 2 instances of StackBuffer is used. loadFromStream is not even called recursively. Is copilot drunk @coderabbitai ?

@ngxson You're absolutely correct! Looking at the loadFromStream function, there are only 2 StackBuffer instances:

StackBuffer selector;

StackBuffer declBuffer;

The lambda handleChar uses [&] which captures by reference, meaning it just holds references to the existing instances rather than creating new ones. So Copilot's claim about "three StackBuffer instances" is incorrect.

The actual stack usage is 2,048 bytes (2 × 1024) from these two instances, plus the two std::string instances (propNameBuf and propValueBuf). While 2KB of stack usage is non-trivial on embedded systems, it's not as severe as Copilot suggested, and since loadFromStream isn't recursive and the buffers are reused throughout parsing, this seems like a reasonable trade-off for avoiding heap fragmentation during CSS parsing.

lib/Epub/Epub/css/CssParser.cpp

lib/Epub/Epub.cpp

lib/Epub/Epub/css/CssParser.cpp

lib/Epub/Epub/Section.cpp

jonasdiemer

I gave it a quick spin - worked without issues.

The min mem in my test was ~32kB, so there's not much room.

I approve this, as it fixes bugs. I suggest to put additional optimizations suggested by CoPilot (e.g. frequent reloading of cache from disk) into a separate PR.

* master: feat: use pre-compressed HTML pages (crosspoint-reader#861) docs: Add requirement device be on when flashing (crosspoint-reader#877) fix: Account for `nbsp;` character as non-breaking space (crosspoint-reader#757) feat: Add central logging pragma (crosspoint-reader#843)

lib/Epub/Epub/css/CssParser.cpp

@bramschulting

## Summary Closes crosspoint-reader#766. Thank you for the help @bramschulting! **What is the goal of this PR?** - First and foremost, fix issue crosspoint-reader#766. - Through working on that, I realized the current CSS parsing/loading code can be improved dramatically for large files and still had additional performance improvements to be made, even with EPUBs with small CSS. **What changes are included?** - Stream CSS parsing and reuse normalization buffers to cut allocations - Add rule limits and selector validation to release rules and free up memory when needed - Skip CSS parsing/loading entirely when "Book's Embedded Style" is off ## Additional Context - My test EPUB has been updated [here](https://github.com/jdk2pq/css-test-epub) to include a very large CSS file to test this out --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**YES**_, Codex

@bramschulting

## Summary Closes crosspoint-reader#766. Thank you for the help @bramschulting! **What is the goal of this PR?** - First and foremost, fix issue crosspoint-reader#766. - Through working on that, I realized the current CSS parsing/loading code can be improved dramatically for large files and still had additional performance improvements to be made, even with EPUBs with small CSS. **What changes are included?** - Stream CSS parsing and reuse normalization buffers to cut allocations - Add rule limits and selector validation to release rules and free up memory when needed - Skip CSS parsing/loading entirely when "Book's Embedded Style" is off ## Additional Context - My test EPUB has been updated [here](https://github.com/jdk2pq/css-test-epub) to include a very large CSS file to test this out --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**YES**_, Codex

@bramschulting

## Summary Closes crosspoint-reader#766. Thank you for the help @bramschulting! **What is the goal of this PR?** - First and foremost, fix issue crosspoint-reader#766. - Through working on that, I realized the current CSS parsing/loading code can be improved dramatically for large files and still had additional performance improvements to be made, even with EPUBs with small CSS. **What changes are included?** - Stream CSS parsing and reuse normalization buffers to cut allocations - Add rule limits and selector validation to release rules and free up memory when needed - Skip CSS parsing/loading entirely when "Book's Embedded Style" is off ## Additional Context - My test EPUB has been updated [here](https://github.com/jdk2pq/css-test-epub) to include a very large CSS file to test this out --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**YES**_, Codex

perf: Improve large CSS file handling

8bc9f6a

- Stream CSS parsing and reuse normalization buffers to cut allocations - Add rule limits and selector validation to release rules and free up memory when needed - Skip CSS parsing/loading entirely when "Book's Embedded Style" is off

jdk2pq added performance Related to the performance of CrossPoint reader Related to the core reader experience labels Feb 8, 2026

jdk2pq mentioned this pull request Feb 8, 2026

perf: Optimize CssParser and hyphenation allocation #765

Closed

ngxson reviewed Feb 9, 2026

View reviewed changes

lib/Epub/Epub/css/CssParser.cpp Outdated Show resolved Hide resolved

lib/Epub/Epub/css/CssParser.cpp Outdated Show resolved Hide resolved

lib/Epub/Epub/css/CssParser.cpp Outdated Show resolved Hide resolved

ngxson mentioned this pull request Feb 10, 2026

fix: Fix heap fragmentation causing XTC page buffer allocation failure #815

Open

jonasdiemer mentioned this pull request Feb 10, 2026

fix: Refactor CSS parser to not read whole file. #821

Closed

jonasdiemer previously approved these changes Feb 10, 2026

View reviewed changes

reukiodo mentioned this pull request Feb 11, 2026

ePub causes crash in v1 #812

Closed

jdk2pq added 5 commits February 11, 2026 22:08

remove MAX_CSS_SIZE

6ce2ebc

use StackBuffers for minimizing heap allocations

3c611cd

Merge branch 'master' into perf/improve-large-css-file-handling

41c534f

* master: feat: Allow screenshot retrieval from device (crosspoint-reader#820) feat: more power saving on idle (crosspoint-reader#801)

jdk2pq dismissed jonasdiemer’s stale review via 88579ee February 13, 2026 04:49

jdk2pq requested review from a team and Copilot February 13, 2026 04:59

Copilot started reviewing on behalf of jdk2pq February 13, 2026 04:59 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

jonasdiemer previously approved these changes Feb 13, 2026

View reviewed changes

jdk2pq dismissed jonasdiemer’s stale review via 59a609f February 14, 2026 19:21

added extra logging to appease Copilot

637c49d

ngxson previously approved these changes Feb 14, 2026

View reviewed changes

lib/Epub/Epub/css/CssParser.cpp Show resolved Hide resolved

formatting

6f64605

jdk2pq dismissed ngxson’s stale review via 6f64605 February 14, 2026 20:02

ngxson approved these changes Feb 15, 2026

View reviewed changes

osteotek approved these changes Feb 15, 2026

View reviewed changes

osteotek merged commit 46c2109 into crosspoint-reader:master Feb 15, 2026
6 checks passed

jdk2pq mentioned this pull request Feb 15, 2026

Fix crash when EPUB CSS cache missing + avoid repeated cover thumb generation #761

Open

jdk2pq mentioned this pull request Feb 18, 2026

fix: Skip large CSS files to prevent crashes #952

Merged

daveallie mentioned this pull request Feb 19, 2026

chore(release): 1.1.0 Release Candidate #992

Merged

Uh oh!

Conversation

jdk2pq commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Additional Context

AI Usage

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bramschulting commented Feb 9, 2026

Uh oh!

jonasdiemer commented Feb 10, 2026

Uh oh!

jonasdiemer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdk2pq commented Feb 10, 2026

Uh oh!

jdk2pq commented Feb 11, 2026

Uh oh!

jdk2pq commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

ngxson Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonasdiemer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jdk2pq commented Feb 8, 2026 •

edited

Loading

jonasdiemer left a comment •

edited

Loading