Skip to content

Update closure-compiler. NFC - #22517

Merged
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:update_closure
Sep 6, 2024
Merged

Update closure-compiler. NFC#22517
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:update_closure

Conversation

@sbc100

@sbc100 sbc100 commented Sep 5, 2024

Copy link
Copy Markdown
Collaborator

There is tiny codesize regression with this update, but it seems to be
consistently <10 bytes so I'm tempted to ignore it.

@sbc100
sbc100 requested a review from dschuff September 5, 2024 20:39
@sbc100

sbc100 commented Sep 5, 2024

Copy link
Copy Markdown
Collaborator Author

There is tiny codesize regression with this update, but it seems to be
consistently <10 bytes so I'm tempted to ignore it.

@sbc100
sbc100 requested a review from kripken September 5, 2024 20:53
@sbc100

sbc100 commented Sep 5, 2024

Copy link
Copy Markdown
Collaborator Author

Looking at the diff for test_codesize_hello_wasmfs it looks like this:

-    T = "undefined" != typeof TextDecoder ? new TextDecoder : void 0, ja = {
-      b : (a, c, d) => C.copyWithin(a, c, c + d),
+    U = "undefined" != typeof TextDecoder ? new TextDecoder : void 0, ja = {
+      b : (a, c, d) => D.copyWithin(a, c, c + d),
       a : (a, c, d, e) => {
-        for (var n = 0, W = 0; W < d; W++) {
-          var la = D[c >> 2], X = D[c + 4 >> 2];
+        for (var n = 0, X = 0; X < d; X++) {
+          var ma = E[c >> 2], Y = E[c + 4 >> 2];
           c += 8;
-          for (var H = 0; H < X; H++) {
-            var f = C[la + H], I = ia[a];
+          for (var I = 0; I < Y; I++) {
+            var z = a, f = D[ma + I], J = ia[z];
             if (0 === f || 10 === f) {
-              f = I;
+              z = 1 === z ? ba : y;
+              f = J;
               for (var k = 0, m = k + NaN, q = k; f[q] && !(q >= m);)
                 ++q;
-              if (16 < q - k && f.buffer && T)
-                f = T.decode(f.subarray(k, q));
+              if (16 < q - k && f.buffer && U)
+                f = U.decode(f.subarray(k, q));

Note the extra z = 1 === z ? ba : y; in the new code.

Seems to be one of the UTF8 functions from library_strings.js.. just its hard to figure out what exactly is going on here.

@dschuff

dschuff commented Sep 5, 2024

Copy link
Copy Markdown
Member

Yeah, seems fine to me. better to be up to date.

There is tiny codesize regression with this update, but it seems to be
consistently <10 bytes so I'm tempted to ignore it.
@sbc100

sbc100 commented Sep 6, 2024

Copy link
Copy Markdown
Collaborator Author

@kripken WDYT?

@kripken kripken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense, an upstream change this small is probably not worth spending time on.

@sbc100
sbc100 enabled auto-merge (squash) September 6, 2024 17:55
@sbc100
sbc100 disabled auto-merge September 6, 2024 17:55
@sbc100
sbc100 enabled auto-merge (squash) September 6, 2024 17:57
@sbc100
sbc100 merged commit 972bf6c into emscripten-core:main Sep 6, 2024
@sbc100
sbc100 deleted the update_closure branch September 6, 2024 18:12
@reneleonhardt

Copy link
Copy Markdown

google/closure-compiler-npm#314
has been released May 15, can you update to v20250226.0.0 to allow building on macOS arm64?

sql-js/sql.js#613

building:WARNING: falling back to java version of closure compiler
Command '['/emsdk/node/20.18.0_64bit/bin/node', '--max_old_space_size=8192', '/emsdk/upstream/emscripten/node_modules/.bin/google-closure-compiler', '--platform=java', '--version']' returned non-zero exit status 254.
emcc: error: closure compiler (/emsdk/node/20.18.0_64bit/bin/node --max_old_space_size=8192 /emsdk/upstream/emscripten/node_modules/.bin/google-closure-compiler --platform=java --version) did not execute properly!

@sbc100

sbc100 commented Aug 29, 2025

Copy link
Copy Markdown
Collaborator Author

We ran into some regressions when trying to update so we are currently blocked on investigating that.

@sbc100

sbc100 commented Aug 29, 2025

Copy link
Copy Markdown
Collaborator Author

See #24679

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.

4 participants