Skip to content
This repository was archived by the owner on Jan 29, 2026. It is now read-only.

feat: Adopt upstream PRs #2867/#1929 - Performance optimizations#4

Open
nfcampos wants to merge 1 commit into
masterfrom
claude/analyze-group-19-prs-0168a9zo8oFGcHyVQxgpSHZC
Open

feat: Adopt upstream PRs #2867/#1929 - Performance optimizations#4
nfcampos wants to merge 1 commit into
masterfrom
claude/analyze-group-19-prs-0168a9zo8oFGcHyVQxgpSHZC

Conversation

@nfcampos
Copy link
Copy Markdown

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:

Summary

Test plan

Related to source code (for typings update)

…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
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +287 to +291
switch (this.cacheMode) {
case StyleCacheMode.FAST_MAP:
cacheKey = encodeStyle(model);
if (this.styleCache.has(cacheKey)) {
return this.styleCache.get(cacheKey);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants