Skip to content

Improve build performance - #105

Merged
mathiasbynens merged 1 commit into
node-unicode:mainfrom
JLHwung:improve-build-performance
Sep 10, 2026
Merged

mathiasbynens merged 1 commit into
node-unicode:mainfrom
JLHwung:improve-build-performance

Conversation

@JLHwung

@JLHwung JLHwung commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

This PR includes commits from #104, please review that PR first. I will rebase once that PR gets merged.

In abf6544 we have replaced sync fs I/O with sequential async fs I/O, which is less efficient because of the async overhead.

In this PR we reverted the async fs I/O changes and as we can see the build time has been improved from 20s to 14s, a 1.4x performance gain.

Runtime measured in my local environment (Apple M1 Max 10c)

# Current main:
$ time npm run build
npm run build  19.99s user 43.52s system 435% cpu 14.588 total

# This PR:
$ time npm run build
npm run build  13.55s user 37.18s system 343% cpu 14.782 total

@mathiasbynens

Copy link
Copy Markdown
Collaborator

LGTM but I’ll wait for the rebase. I do prefer the way to code looks with async, but the performance wins are worth it in this case.

19.99s → 12.87s
@JLHwung
JLHwung force-pushed the improve-build-performance branch from f9b4f6d to 647ebc3 Compare September 10, 2026 12:30
@JLHwung

JLHwung commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased.

CPU profile shows that 80% of the running time is spent on the writeFileUtf8 native binding, so the build script is heavily IO-bounded. Any inefficiency in IO operations will significantly increase the build time.

Compared with two years ago, the running time has increased from 10s to 14s, probably due to the d.mts emits. No complaints here as the d.mts files are useful.

@mathiasbynens
mathiasbynens merged commit 1fed9fe into node-unicode:main Sep 10, 2026
1 check passed
@mathiasbynens

Copy link
Copy Markdown
Collaborator

Thanks!

@JLHwung
JLHwung deleted the improve-build-performance branch September 10, 2026 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants