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

worker.js file should be placed into the output directory when targeting Emscripten with pthread support #13720

Closed
syrel opened this issue Apr 7, 2024 · 10 comments
Labels
A-layout Area: target output directory layout, naming, and organization C-bug Category: bug O-wasm OS: WASM target released issues S-propose-close Status: A team member has nominated this for closing, pending further input from the team

Comments

@syrel
Copy link

syrel commented Apr 7, 2024

Problem

When targeting wasm32-unknown-emscripten with Pthread support, the *.worker.js file is not copied to the output directory from the deps folder, even though the Emscripten compiler generates that file. The worker.js is required by multithreaded wasm apps and is a mandatory output artefact.

Steps

# Create a test package
cargo new --bin issue && cd issue
# Compile with Pthreads support
cargo +nightly \
   -Z build-std=panic_abort,std \
   --config='build.rustflags = ["-C","target-feature=+atomics,+bulk-memory", "-C", "link-args=-pthread"]' \
   build \
      --bin issue \
      --target wasm32-unknown-emscripten
# List output files
find target/wasm32-unknown-emscripten/debug -maxdepth 1 -regex ".*issue.*"

Expected output:

target/wasm32-unknown-emscripten/debug/issue.d
target/wasm32-unknown-emscripten/debug/issue.js
target/wasm32-unknown-emscripten/debug/issue.wasm
target/wasm32-unknown-emscripten/debug/issue.worker.js

Actual output:

target/wasm32-unknown-emscripten/debug/issue.d
target/wasm32-unknown-emscripten/debug/issue.js
target/wasm32-unknown-emscripten/debug/issue.wasm

Note that issue.worker.js is missing in the output directory. However, it is successfully created by the compiler and can be found in the deps folder:

$ find target/wasm32-unknown-emscripten/debug/deps -maxdepth 1 -regex ".*issue.*"

target/wasm32-unknown-emscripten/debug/deps/issue.d
target/wasm32-unknown-emscripten/debug/deps/issue.js
target/wasm32-unknown-emscripten/debug/deps/issue.wasm
target/wasm32-unknown-emscripten/debug/deps/issue.worker.js

Possible Solution(s)

A solution would be to adjust the list of file types returned by file_types() for wasm32 target to include *.worker.js:

if target_triple.starts_with("wasm32-") && crate_type == CrateType::Bin && suffix == ".js" {

Notes

No response

Version

cargo 1.77.1 (e52e36006 2024-03-26)
release: 1.77.1
commit-hash: e52e360061cacbbeac79f7f1215a7a90b6f08442
commit-date: 2024-03-26
host: aarch64-apple-darwin
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.4.0 (sys:0.4.70+curl-8.5.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 14.3.1 [64-bit]
@syrel syrel added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Apr 7, 2024
@weihanglo weihanglo added the O-wasm OS: WASM target released issues label Apr 7, 2024
@weihanglo
Copy link
Member

Thanks for the report!

Could you share some more specifications and documentations of that output behavior of emscripten? I found one here, but not sure if it always does that. worker.js could possibly be considered as an auxiliary file and uplifted for empscripten target, though we might need to understand the stability before moving forward. We might also bump into issues like #13355 and #13358 when dealing with it.

@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. A-layout Area: target output directory layout, naming, and organization and removed S-triage Status: This issue is waiting on initial triage. labels Apr 7, 2024
@weihanglo
Copy link
Member

BTW, are there any more files for emscripten that we want to uplift to the target directory?

@syrel
Copy link
Author

syrel commented Apr 7, 2024

Hi @weihanglo

Thanks for a quick response! In the Special Considerations section it mentions that the worker.js will be always generated when compiling with -pthread flag and the file must be deployed:

Also note that when compiling code that uses pthreads, an additional JavaScript file NAME.worker.js is generated alongside the output .js file (where NAME is the basename of the main file being emitted). That file must be deployed with the rest of the generated code files.

In addition, -pthread is not the only case when Emscripten generates a worker.js. According to the emcc compiler manual , when linking with --proxy-to-worker flag it will also generate worker.js when emitting JavaScript. Cargo always emits JavaScript and never targets HTML when building for wasm32.

--proxy-to-worker
[link] Runs the main application code in a worker, proxying events to it and output from it. If emitting HTML, this emits a .html file, and a separate .js file containing the JavaScript to be run in a worker. If emitting JavaScript, the target file name contains the part to be run on the main thread, while a second .js file with suffix “.worker.js” will contain the worker portion.

Emscripten choses what to emit based on a suffix of the output file: .js for JavaScript and .html for HTML. The following condition in cargo guarantees that Emscripten will emit JavaScript (due to .js suffix) and therefore we get a worker.js file.

if target_triple.starts_with("wasm32-") && crate_type == CrateType::Bin && suffix == ".js" {

Additional files

BTW, are there any more files for emscripten that we want to uplift to the target directory?

Yes indeed. Those files are: .data, .data.js and .mem. See Build Files. data related files are the important ones. They contain preloaded files and directories that will populate the virtual file system. See Packaging Files. It is difficult to envision an app without asset files. Please consider outputting them as well.

Cheers

@weihanglo weihanglo added S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. labels Apr 8, 2024
@weihanglo
Copy link
Member

Hey. Sorry for the delayed response.

The Cargo team discussed this during today's meeting. The general idea here is that we are okay for adding worker.js as an additional auxiliary files, since we already have special cases for something like .wasm.map.. For any other files at this moment, I'll encourage using --message-format=json to access artifact messages and process what you need if possible.

Just note that, due to the fact that wasm32-unknown-emscripten is a tier-2 target, as well as we lack of CI infra to test it, Cargo would provide its best-effort for the proposed behavior. There is no strong guarantee like tier-1.

In a better future, we'd like to see rustc informing Cargo about the files it emits, so that Cargo doesn't need to hard-code all of them. Thank you.

@weihanglo weihanglo added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels May 7, 2024
@charmitro
Copy link
Contributor

A general note for the future, keep in mind of this PR that recently was merged in Emscripten.

With the master branch Emcscripten, *.worker.js file generates the following,

// This file is no longer used by emscripten and has been created as a placeholder                                                                                                           
// to allow build systems to transition away from depending on it.                                                                                                                           
//                                                                                                                                                                                           
// Future versions of emscripten will likely stop generating this file at all.

@weihanglo
Copy link
Member

Thanks @charmitro for the updated info!

Given *.worker.js is no longer useful, I propose to close this issue.

@weihanglo weihanglo added S-propose-close Status: A team member has nominated this for closing, pending further input from the team and removed S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review labels May 22, 2024
@charmitro
Copy link
Contributor

Thanks @charmitro for the updated info!

Given *.worker.js is no longer useful, I propose to close this issue.

I generally agree. However, is there a way to check which version is installed and, based on that, generate *.worker.js, at least until Emscripten completely removes this feature?

@weihanglo
Copy link
Member

To me, I won't bother that. Emscripten is not a tier 1 platform and user can always download the latest version of it. The maintenance cost might not be worthy for a deprecated-soon-removed feature. And we don't have proper CI infra to test that. Also, I don't think there is a safe and stable way to get the version info. Invoking emscripnt is probably the straightforward one, and Cargo should invoke external binaries as less as possible.

@charmitro
Copy link
Contributor

Makes sense, you're right. I completely agree with not moving forward with this.

@syrel
Copy link
Author

syrel commented May 27, 2024

Hi

What a timing of the emscripten change! Thanks @charmitro for digging it up. I think we an close the issue 👍

Cheers

@syrel syrel closed this as completed May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Area: target output directory layout, naming, and organization C-bug Category: bug O-wasm OS: WASM target released issues S-propose-close Status: A team member has nominated this for closing, pending further input from the team
Projects
None yet
Development

No branches or pull requests

3 participants