src: compress ICU data into a shared cache file by anonrig · Pull Request #66211 · nodejs/node

anonrig · 2026-09-22T20:11:27Z

Depends on #66186. The ICU-only diff is compress-embedded-blobs...compress-icu-data.

Why

Full ICU data is 33 MB of clean, shareable pages in the binary. Those pages should stay that way. The bytes stored in the executable do not have to be the raw file. Whether that tradeoff is worth it is a packager decision, so it is off unless requested.

What

./configure --with-icu-compress stores the data file as a zstd frame named icudt<version><endian>.dat.z (33,105,536 bytes down to 9,223,488, header included). The entry point stays an ICU data symbol, icudt<version><endian>_dat_zstd_dat.

The first process inflates it into icudt78l-<sha256>.dat under the temp directory, mode 0600, and maps that file read-only. Later processes map the same file. Before the mapping is handed to ICU, the bytes are checked against the SHA-256 stored in the binary. A file that does not match is deleted and written again. If the temp directory cannot be written, startup keeps the private buffer so ICU still runs. --icu-data-dir still wins when it is set.

Without the flag, ICU data stays the normal linked file.

Store the full ICU data file, one-byte builtin sources, and the V8 startup snapshot plus code caches as zstd frames instead of raw bytes. Decompress each once at startup and keep it for the process lifetime. On macOS Release, link the executable with -dead_strip, -x, and -S. A Release arm64 macOS binary dropped from 140 MB to 59 MB. Intl, builtins, the snapshot, and code cache behavior are unchanged. Startup RSS rose by about 40 MB because those blobs are private heap instead of file-backed pages. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>

Wrap lines past 80 columns and store the zstd frame size in uint64_t. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>

-Wl,-dead_strip on the Release executable removed napi_* and other globals that nothing in the executable calls. Addons resolve those symbols from the host, so doc-kit's lightningcss addon called a null pointer inside napi_register_module_v1 and macOS CI died with SIGSEGV while generating docs. -x and -S stay; they only drop local and debug symbols. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok

udata_setCommonData() stores the package address and later lookups return pointers into that storage, so the decompressed buffer cannot be freed. Inflating the full ICU data file at startup turned every page into private dirty memory. Map the original data from the executable again so unused pages stay demand-paged, clean, and shareable across processes. Builtin sources and the startup snapshot remain compressed. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok

The ICU data file is a zstd frame in the binary. The first process inflates it into a file under the temp directory and maps that file read-only. Later processes map the same file, so the pages stay clean, demand-paged, and shared instead of a private dirty copy. If the temp directory cannot be written, startup keeps the private buffer so ICU still works. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok

Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok

nodejs-github-bot · 2026-09-22T20:11:33Z

Review requested:

@nodejs/gyp
@nodejs/security-wg
@nodejs/startup

srl295

I think this is interesting and i could support it, but it absolutely must be opt in so packagers need to make the decision about whether they want to pay for the startup cost and complexity in exchange for on-disk footprint.

srl295 · 2026-09-22T21:14:00Z

+                        '<(PRODUCT_DIR)/zstd_compress<(EXECUTABLE_SUFFIX)',
+                        '<(icu_data_in)',
+                      ],
+                      'outputs': [ '<(SHARED_INTERMEDIATE_DIR)/node_icu_zstd.dat' ],


Don't drop the version number and endianness here.

Instead name the file icudt78l.dat.z or something (add a suffix).

This will cause a lot of harm trying to figure out which version of the data file is being used.

srl295 · 2026-09-22T21:15:35Z

                                  '-d', '<(SHARED_INTERMEDIATE_DIR)',
                                  '-n', 'icudata',
-                                  '-e', 'icudt<(icu_ver_major)',
+                                  '-e', 'node_icu_zstd',


disruptive and unnecessary to change the entry point here, making it unrelated to icu

Suggested change

'-e', 'node_icu_zstd',

'-e', 'icudt<(icu_ver_major)<(icu_endianness)_dat_zstd',

srl295 · 2026-09-22T21:21:04Z

Repeating (here) my suggestion to discuss this upstream with ICU, that's where it should be implemented.

See https://unicode-org.atlassian.net/browse/ICU-6429 for example. I would support that. As I noted, I'm working on an adhoc on platform cost of supporting more and more languages in CLDR consumers (such as ICU, such as CLDR).

akx · 2026-09-22T21:34:34Z

+    return nullptr;
+  }
+  std::string path;
+  if (CachePath(expect_hash, &path)) {


Am I missing something, or is the integrity of the cached data on disk not checked before it's mapped in?

I noticed it's created mode 0600, but processes run by the user could still tamper with it to unexpected results, right? Or does the ICU have internal integrity checks for udata_setCommonData data?

Magic numbers are checked and sizes checked, but no the data is not checksummed etc. That would be part of an install process.

But this file is written here by node into a temp directory, right? No install process there...

IOW, what I'm thinking is:

node starts, notices no cache file, decompresses ICU data into cache file, does whatever, exits

something tampers with the cache file

node starts, notices cache file, maps it into memory... and ICU is confused and crashes or worse

Always possible but given the node.js trust model (any code that runs is trusted) that really wouldn't be considered a vulnerability. Given how ICU data is used, it wouldn't be considered a high risk.

Packagers pass --with-icu-compress. The default build keeps the data file mapped from the binary. The compressed blob is named icudt<version><endian>.dat.z and its symbol stays an ICU data name. The cache file is checked against the hash stored in the binary before ICU is pointed at it. A rewritten file is discarded and built again. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok

codecov · 2026-09-22T23:28:20Z

Codecov Report

❌ Patch coverage is 78.08219% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.26%. Comparing base (67a4416) to head (248ac4c).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
src/zstd_blob.cc	45.00%	6 Missing and 5 partials ⚠️
src/node_union_bytes.h	66.66%	2 Missing and 2 partials ⚠️
src/node_snapshotable.cc	97.56%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66211      +/-   ##
==========================================
- Coverage   90.28%   90.26%   -0.03%     
==========================================
  Files         790      791       +1     
  Lines      272044   272900     +856     
  Branches    51949    52119     +170     
==========================================
+ Hits       245625   246328     +703     
- Misses      16915    17023     +108     
- Partials     9504     9549      +45

Files with missing lines	Coverage Δ
src/node_i18n.cc	`78.27% <ø> (ø)`
src/node_snapshotable.cc	`73.67% <97.56%> (-0.07%)`	⬇️
src/node_union_bytes.h	`82.60% <66.66%> (-17.40%)`	⬇️
src/zstd_blob.cc	`45.00% <45.00%> (ø)`

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

srl295

It's conditional.. but i don't think this is mergeable. Let's say it's an interesting experiment.

srl295 · 2026-09-22T23:38:26Z

+  if options.with_icu_compress and with_intl not in ('small-icu', 'full-icu'):
+    error('--with-icu-compress requires --with-intl=full-icu or small-icu')


This would be better done in the if system-icu branch.

srl295 · 2026-09-22T23:40:08Z

+      'toolsets': ['host'],
+      'dependencies': ['zstd#host'],
+      'include_dirs': ['lib'],
+      'sources': ['../../tools/zstd_compress.cc'],


I do wonder if bringing in a whole new implementation file is the best option. Isn't there any compression anywhere linked into node already?

srl295 · 2026-09-22T23:41:10Z

- *  - when NOT in NODE_HAVE_SMALL_ICU mode, ICU is linked directly with its full
- *    data. All of the variables and command line options for changing data at
- *    runtime are disabled, as they wouldn't fully override the internal data.
+ *  - Full and small ICU data are stored as a zstd frame in the binary.


Out of date, needs to mention the 'if'

srl295 · 2026-09-22T23:41:19Z

            'WARNING_CFLAGS': [ '-Werror' ],
          },
        }],
+        # The Release executable's local symbol table and STABS dominate


Seems unrelated to ICU.

srl295 · 2026-09-22T23:44:33Z

+#if !defined(MAP_ANON) && defined(MAP_ANONYMOUS)
+#define MAP_ANON MAP_ANONYMOUS
+#endif
+  void* ptr = mmap(


I'm sorry, this is very messy within Node. This should be handled upstream in ICU, which already has its own abstraction for mmap.

srl295 · 2026-09-22T23:45:55Z

 #include "node_v8_platform-inl.h"
 #include "simdjson.h"
 #include "timers.h"
+#include "zstd.h"


Does this belong in this PR?

anonrig added 6 commits September 21, 2026 16:15

src: fix lint on compressed startup blobs

b094ea9

Wrap lines past 80 columns and store the zstd frame size in uint64_t. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>

src: fix clang-format in ICU cache helpers

8e075c9

Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok

anonrig mentioned this pull request Sep 22, 2026

src: compress embedded icu, builtins, and snapshot #66186

Open

nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 22, 2026

srl295 requested changes Sep 22, 2026

View reviewed changes

akx reviewed Sep 22, 2026

View reviewed changes

srl295 requested changes Sep 22, 2026

View reviewed changes

	'-e', 'node_icu_zstd',
	'-e', 'icudt<(icu_ver_major)<(icu_endianness)_dat_zstd',

		if options.with_icu_compress and with_intl not in ('small-icu', 'full-icu'):
		error('--with-icu-compress requires --with-intl=full-icu or small-icu')

Uh oh!

Conversation

anonrig commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 22, 2026

Uh oh!

srl295 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srl295 commented Sep 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 22, 2026

Codecov Report

Uh oh!

srl295 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anonrig commented Sep 22, 2026 •

edited

Loading

srl295 left a comment •

edited

Loading