Skip to content

chore: Bump terser #4791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

chore: Bump terser #4791

wants to merge 3 commits into from

Conversation

rschristian
Copy link
Member

@rschristian rschristian commented Jun 7, 2025

Terser was locked in #4687 as we saw significant perf degradation from bumping the version further. Upon further investigation, it turns out this was due to a lack of hoisting:

In our CJS & ESM builds, NULL & UNDEFINED are not hoisted as constants but inlined throughout the bundle. For reasons that I still don't quite understand, hoisting these two, as newer versions of Terser will do, is a huge perf issue for us, causing double-digit regressions across the board. The byte impact is completely overshadowed by the perf hit, so, I've removed the NULL & UNDEFINED constants. We'll go back to using null & undefined/void 0 throughout the source.

Namespaces are a similar story; right now they're inlined, newer versions of Terser will hoist, but this is a very tiny byte reduction (we reference Math only once and XHTML & SVG twice each) with a 1-4% perf degradation in a few tests. Inlining them too seems better.

Edit: The perf regression is much larger w/ JIT enabled, disable it & hoisting is a slight improvement in some, still a regression in others. Not that it matters much ofc, just an interesting tidbit.


Therefore, the only real changes of this PR are as follows:

  • Switches us to patterns that match our desired output/perf characteristics w/ modern Terser
  • Matches the UMD build to our CJS & ESM builds
    • It's probably safe to assume it largely matches the performance characteristics of the CJS & ESM builds, and if so, it's likely slower than it should be due to the hoisting that only it is receiving at the moment.
  • Compat's version is now hoisted too, instead of inlined into the CJS exports. Harder to fix (or at least, I'm not sure how to) as we don't easily control how the exports are generated.
    • i.e., exports.version = '18.3.1' has become exports.version = P

No worries if we want to just stick w/ older Terser, I mostly did this as an exploration as I was curious. There's no definite advantage here (I could go bench UMD builds I suppose though), just refactoring us so new Terser lines up with our old Terser output.

@rschristian rschristian changed the title junk: Bump tersers, hoists namespaces, NULL, & UNDEFINED junk: Bump terser, hoists namespaces, NULL, & UNDEFINED Jun 7, 2025
Copy link

github-actions bot commented Jun 7, 2025

📊 Tachometer Benchmark Results

Summary

duration

  • create10k: unsure 🔍 -1% - +1% (-8.27ms - +8.45ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -0% - +0% (-0.04ms - +0.03ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -1% - +3% (-0.41ms - +2.28ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -4% - +3% (-0.68ms - +0.50ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -1% - +3% (-0.77ms - +1.90ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -7% - +0% (-0.15ms - +0.01ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -2% - +1% (-0.64ms - +0.41ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -4% - +2% (-1.15ms - +0.61ms)
    preact-local vs preact-main

usedJSHeapSize

  • create10k: unsure 🔍 -0% - -0% (-0.00ms - -0.00ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -0% - -0% (-0.01ms - -0.00ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -8% - +0% (-0.65ms - +0.02ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -0% - -0% (-0.01ms - -0.00ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -0% - -0% (-0.01ms - -0.00ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -3% - +1% (-0.03ms - +0.01ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -2% - +0% (-0.02ms - +0.01ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -0% - +0% (-0.01ms - +0.00ms)
    preact-local vs preact-main

Results

create10k

duration

VersionAvg timevs preact-localvs preact-main
preact-local986.41ms - 999.17ms-unsure 🔍
-1% - +1%
-8.27ms - +8.45ms
preact-main987.30ms - 998.10msunsure 🔍
-1% - +1%
-8.45ms - +8.27ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local19.18ms - 19.18ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms
preact-main19.18ms - 19.18msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms
-
filter-list

duration

VersionAvg timevs preact-localvs preact-main
preact-local16.53ms - 16.58ms-unsure 🔍
-0% - +0%
-0.04ms - +0.03ms
preact-main16.53ms - 16.58msunsure 🔍
-0% - +0%
-0.03ms - +0.04ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.53ms - 1.53ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms
preact-main1.53ms - 1.54msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms
-
hydrate1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local67.92ms - 70.11ms-unsure 🔍
-1% - +3%
-0.41ms - +2.28ms
preact-main67.30ms - 68.86msunsure 🔍
-3% - +1%
-2.28ms - +0.41ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local7.18ms - 7.65ms-unsure 🔍
-8% - +0%
-0.65ms - +0.02ms
preact-main7.49ms - 7.97msunsure 🔍
-0% - +9%
-0.02ms - +0.65ms
-
many-updates

duration

VersionAvg timevs preact-localvs preact-main
preact-local15.87ms - 16.41ms-unsure 🔍
-4% - +3%
-0.68ms - +0.50ms
preact-main15.71ms - 16.76msunsure 🔍
-3% - +4%
-0.50ms - +0.68ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local3.77ms - 3.77ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms
preact-main3.77ms - 3.78msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms
-
replace1k
  • Browser: chrome-headless
  • Sample size: 100
  • Built by: CI #4769
  • Commit: ff3b892

duration

VersionAvg timevs preact-localvs preact-main
preact-local66.72ms - 68.93ms-unsure 🔍
-1% - +3%
-0.77ms - +1.90ms
preact-main66.52ms - 68.00msunsure 🔍
-3% - +1%
-1.90ms - +0.77ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local2.98ms - 2.98ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms
preact-main2.98ms - 2.99msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms
-

run-warmup-0

VersionAvg timevs preact-localvs preact-main
preact-local26.33ms - 26.99ms-unsure 🔍
-3% - +1%
-0.88ms - +0.14ms
preact-main26.64ms - 27.42msunsure 🔍
-1% - +3%
-0.14ms - +0.88ms
-

run-warmup-1

VersionAvg timevs preact-localvs preact-main
preact-local33.40ms - 34.81ms-unsure 🔍
-4% - +2%
-1.34ms - +0.69ms
preact-main33.71ms - 35.16msunsure 🔍
-2% - +4%
-0.69ms - +1.34ms
-

run-warmup-2

VersionAvg timevs preact-localvs preact-main
preact-local29.79ms - 30.84ms-unsure 🔍
-3% - +2%
-0.87ms - +0.58ms
preact-main29.96ms - 30.96msunsure 🔍
-2% - +3%
-0.58ms - +0.87ms
-

run-warmup-3

VersionAvg timevs preact-localvs preact-main
preact-local27.71ms - 28.73ms-unsure 🔍
-3% - +2%
-0.85ms - +0.61ms
preact-main27.82ms - 28.86msunsure 🔍
-2% - +3%
-0.61ms - +0.85ms
-

run-warmup-4

VersionAvg timevs preact-localvs preact-main
preact-local22.81ms - 24.03ms-unsure 🔍
-2% - +5%
-0.56ms - +1.15ms
preact-main22.53ms - 23.72msunsure 🔍
-5% - +2%
-1.15ms - +0.56ms
-

run-final

VersionAvg timevs preact-localvs preact-main
preact-local22.19ms - 23.18ms-unsure 🔍
-0% - +6%
-0.03ms - +1.24ms
preact-main21.69ms - 22.48msunsure 🔍
-5% - +0%
-1.24ms - +0.03ms
-
text-update
  • Browser: chrome-headless
  • Sample size: 230
  • Built by: CI #4769
  • Commit: ff3b892

duration

VersionAvg timevs preact-localvs preact-main
preact-local2.09ms - 2.19ms-unsure 🔍
-7% - +0%
-0.15ms - +0.01ms
preact-main2.15ms - 2.27msunsure 🔍
-0% - +7%
-0.01ms - +0.15ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.15ms - 1.18ms-unsure 🔍
-3% - +1%
-0.03ms - +0.01ms
preact-main1.15ms - 1.19msunsure 🔍
-1% - +3%
-0.01ms - +0.03ms
-
todo

duration

VersionAvg timevs preact-localvs preact-main
preact-local32.90ms - 33.22ms-unsure 🔍
-2% - +1%
-0.64ms - +0.41ms
preact-main32.67ms - 33.68msunsure 🔍
-1% - +2%
-0.41ms - +0.64ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.24ms - 1.25ms-unsure 🔍
-2% - +0%
-0.02ms - +0.01ms
preact-main1.24ms - 1.26msunsure 🔍
-0% - +2%
-0.01ms - +0.02ms
-
update10th1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local29.93ms - 31.11ms-unsure 🔍
-4% - +2%
-1.15ms - +0.61ms
preact-main30.14ms - 31.44msunsure 🔍
-2% - +4%
-0.61ms - +1.15ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local2.93ms - 2.94ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms
preact-main2.94ms - 2.94msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms
-

tachometer-reporter-action v2 for CI

Copy link

github-actions bot commented Jun 7, 2025

Size Change: +13 B (+0.03%)

Total Size: 46.6 kB

Filename Size Change
compat/dist/compat.js 3.78 kB +4 B (+0.11%)
compat/dist/compat.umd.js 3.83 kB +1 B (+0.03%)
dist/preact.umd.js 4.73 kB +8 B (+0.17%)
ℹ️ View Unchanged
Filename Size
compat/dist/compat.mjs 3.69 kB
debug/dist/debug.js 3.85 kB
debug/dist/debug.mjs 3.85 kB
debug/dist/debug.umd.js 3.94 kB
devtools/dist/devtools.js 260 B
devtools/dist/devtools.mjs 271 B
devtools/dist/devtools.umd.js 346 B
dist/preact.js 4.65 kB
dist/preact.mjs 4.66 kB
hooks/dist/hooks.js 1.52 kB
hooks/dist/hooks.mjs 1.54 kB
hooks/dist/hooks.umd.js 1.58 kB
jsx-runtime/dist/jsxRuntime.js 861 B
jsx-runtime/dist/jsxRuntime.mjs 830 B
jsx-runtime/dist/jsxRuntime.umd.js 934 B
test-utils/dist/testUtils.js 473 B
test-utils/dist/testUtils.mjs 473 B
test-utils/dist/testUtils.umd.js 555 B

compressed-size-action

@rschristian rschristian changed the title junk: Bump terser, hoists namespaces, NULL, & UNDEFINED junk: Terser testing Jun 7, 2025
@rschristian rschristian changed the title junk: Terser testing chore: Bump terser Jun 7, 2025
Comment on lines +49 to 50
// @ts-expect-error TODO: Legitimate type mismatch that our use of `NULL` hid
oldVNode ? oldVNode._dom : parentDom.firstChild,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a weird one. Above, we have this:

let oldVNode = isHydrating ? NULL : parentDom._children;

For whatever reason, type inference has failed for NULL, so oldVNode was actually any. As such, this is a legitimate (internal) type mismatch in what diff expects as a parameter and what we provide. Will fix separately.

@rschristian rschristian marked this pull request as ready for review June 7, 2025 08:45
Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants