Skip to content
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

[browser] Bump emscripten 2.0.14 #48450

Closed
wants to merge 7 commits into from
Closed

[browser] Bump emscripten 2.0.14 #48450

wants to merge 7 commits into from

Conversation

kjpou1
Copy link
Contributor

@kjpou1 kjpou1 commented Feb 18, 2021

Active issue opened on emcripten : emscripten-core/emscripten#13547
PR open here: emscripten-core/emscripten#13549

Emscripten now builds a complete sysroot inside the EM_CACHE directory. Introduced in 2.0.13

  • This includes the system headers which get copied into place there rather than adding a sequence of extra include directories.

Add -s TEXTDECODER=1 to EMCC_FLAGS to work around bug introduced

ReferenceError: TextDecoder is not defined
    at dotnet.js:1:8517
    at loadScript (runtime.js:196:3)
    at runtime.js:280:1

runtime.js:196: Error executing file
                load (url);
  • Fixes TextDecoder is not defined problem when running tests in console.
info: ReferenceError: TextDecoder is not defined
  info:     at dotnet.js:1:8517
  info:     at loadScript (runtime.js:196:3)
  info:     at runtime.js:280:1
  info:
  info: runtime.js:196: Error executing file
  info:                 load (url);
  info:   ^
  info:
  info: 09:54:03.1461730 Process v8 exited with 1

- This includes the system headers which get copied into place there rather than adding a sequence of extra include directories.
* Default to TEXTDECODER=2 in -Oz builds to save 0.5KB-1KB of build output size. emscripten-core/emscripten#13304
* Fixes `TextDecoder is not defined` problem with running in console as well as when running the AOT console sample.

```
ReferenceError: TextDecoder is not defined
    at dotnet.js:1:8517
    at loadScript (runtime.js:196:3)
    at runtime.js:280:1

runtime.js:196: Error executing file
                load (url);
```

* Fixes `TextDecoder is not defined` problem when running tests in console.

```
info: ReferenceError: TextDecoder is not defined
  info:     at dotnet.js:1:8517
  info:     at loadScript (runtime.js:196:3)
  info:     at runtime.js:280:1
  info:
  info: runtime.js:196: Error executing file
  info:                 load (url);
  info:   ^
  info:
  info: 09:54:03.1461730 Process v8 exited with 1
```
@ghost
Copy link

ghost commented Feb 18, 2021

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

Emscripten now builds a complete sysroot inside the EM_CACHE directory. Introduced in 2.0.13

  • This includes the system headers which get copied into place there rather than adding a sequence of extra include directories.

Add -s TEXTDECODER=0 to EMCC_FLAGS to work around bug introduced

ReferenceError: TextDecoder is not defined
    at dotnet.js:1:8517
    at loadScript (runtime.js:196:3)
    at runtime.js:280:1

runtime.js:196: Error executing file
                load (url);
  • Fixes TextDecoder is not defined problem when running tests in console.
info: ReferenceError: TextDecoder is not defined
  info:     at dotnet.js:1:8517
  info:     at loadScript (runtime.js:196:3)
  info:     at runtime.js:280:1
  info:
  info: runtime.js:196: Error executing file
  info:                 load (url);
  info:   ^
  info:
  info: 09:54:03.1461730 Process v8 exited with 1
Author: kjpou1
Assignees: -
Labels:

area-VM-meta-mono

Milestone: -

@kjpou1 kjpou1 self-assigned this Feb 18, 2021
@kjpou1 kjpou1 added this to the Future milestone Feb 18, 2021
@kjpou1 kjpou1 added the arch-wasm WebAssembly architecture label Feb 18, 2021
@ghost
Copy link

ghost commented Feb 18, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Emscripten now builds a complete sysroot inside the EM_CACHE directory. Introduced in 2.0.13

  • This includes the system headers which get copied into place there rather than adding a sequence of extra include directories.

Add -s TEXTDECODER=0 to EMCC_FLAGS to work around bug introduced

ReferenceError: TextDecoder is not defined
    at dotnet.js:1:8517
    at loadScript (runtime.js:196:3)
    at runtime.js:280:1

runtime.js:196: Error executing file
                load (url);
  • Fixes TextDecoder is not defined problem when running tests in console.
info: ReferenceError: TextDecoder is not defined
  info:     at dotnet.js:1:8517
  info:     at loadScript (runtime.js:196:3)
  info:     at runtime.js:280:1
  info:
  info: runtime.js:196: Error executing file
  info:                 load (url);
  info:   ^
  info:
  info: 09:54:03.1461730 Process v8 exited with 1
Author: kjpou1
Assignees: kjpou1
Labels:

arch-wasm, area-VM-meta-mono

Milestone: Future

@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 18, 2021

Actually thinking this should be TEXTDECODER=1 and not TEXTDECODER=0

TextDecoder is enabled by default, but it is not always used, because it is slower decoding short strings compared to using a manually rolled JavaScript implementation, which is why the current default TEXTDECODER=1 manually decodes short strings, and falls back to TextDecoder for long strings (cutoff is at 16 byte strings).

Also in multithreaded builds TextDecoder cannot be used because .decode() may not allow SAB. The spec was changed at whatwg/encoding#172 to allow it, but it was done in a way that is making hard to have any visibility as to when each browser would have shipped the update. (when we do, we can drop the TEXTDECODER does not support USE_PTHREADS case in the compiler)

…g introduced

- TextDecoder is enabled by default, but it is not always used, because it is slower decoding short strings compared to using a manually rolled JavaScript implementation, which is why the current default `TEXTDECODER=1` manually decodes short strings, and falls back to TextDecoder for long strings (cutoff is at 16 byte strings).
@kjpou1 kjpou1 marked this pull request as ready for review February 18, 2021 10:07
Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

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

looks good but we'll need to wait and bump to the new Docker image once it's available.

src/mono/wasm/Makefile Show resolved Hide resolved
@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 19, 2021

Seems to be issues with System.IO.FileSystem.CopyFile with upgrade to 2.0.13 as well.

System.IO.FileNotFoundException : Unable to find the specified file.

Looking into the cause of change or report issue

@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 19, 2021

Have been able to track the failure to pal_io.c in the copying of the metadata.

With 2.0.12 this returns 0 whereas 2.0.13 returns -1 and errno = 44.

This happened in 2.0.13 so that narrows it down.

@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 22, 2021

This PR is the breaking change: emscripten-core/emscripten#13282

@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 22, 2021

Created issue: emscripten-core/emscripten#13547

@kjpou1 kjpou1 added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 22, 2021
@lewing lewing closed this Feb 24, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-VM-meta-mono NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants