feat: Adopt upstream PRs #2867/#1929 - Performance optimizations#4
feat: Adopt upstream PRs #2867/#1929 - Performance optimizations#4nfcampos wants to merge 1 commit into
Conversation
…mizations Group 19 Analysis: Performance Optimizations PR exceljs#2867 (styleCacheMode): - Adds configurable style caching with 4 modes: - WEAK_MAP: Original behavior (default for backward compatibility) - JSON_MAP: Uses JSON.stringify for cache keys - FAST_MAP: Custom fast serialization (recommended for best performance) - NO_CACHE: Disables caching - FAST_MAP provides ~33% performance improvement for style-heavy workbooks - Works by comparing styles by value instead of object reference - New stylesCacheMode option for xlsx.writeFile() - StyleCacheMode exported from main module PR exceljs#1929 (GC optimization): - Releases large buffers early during file processing - Nullifies chunks, buffer, and zip references after use - Helps GC reclaim memory sooner for large files Benchmark results (heavy_styles - 10000 cells): - WEAK_MAP (default): 186ms - FAST_MAP: 126ms (-32%) Added 18 unit tests for style serialization module. Original PRs: - exceljs#2867 - exceljs#1929
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| switch (this.cacheMode) { | ||
| case StyleCacheMode.FAST_MAP: | ||
| cacheKey = encodeStyle(model); | ||
| if (this.styleCache.has(cacheKey)) { | ||
| return this.styleCache.get(cacheKey); |
There was a problem hiding this comment.
Cache key ignores cell type defaults in FAST_MAP/JSON_MAP
When cacheMode is FAST_MAP or JSON_MAP, addStyleModel derives the cache key from the raw style object before applying the default numFmt derived from the cell’s type. For cells without an explicit numFmt, dates and numbers both hash to the same empty key and the first cached styleId is reused, so one type inherits the other’s default format (e.g., numbers written as mm-dd-yy or dates as General) whenever stylesCacheMode is enabled. The cache key needs to incorporate cellType or be computed after the default numFmt is applied to avoid cross-contaminating styles between value types.
Useful? React with 👍 / 👎.
Group 19 Analysis: Performance Optimizations
PR exceljs#2867 (styleCacheMode):
PR exceljs#1929 (GC optimization):
Benchmark results (heavy_styles - 10000 cells):
Added 18 unit tests for style serialization module.
Original PRs:
Summary
Test plan
Related to source code (for typings update)