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

wasm32-emscripten support #2412

Closed
hoodmane opened this issue May 31, 2022 · 21 comments · Fixed by #2436
Closed

wasm32-emscripten support #2412

hoodmane opened this issue May 31, 2022 · 21 comments · Fixed by #2436

Comments

@hoodmane
Copy link
Contributor

I am interested in wasm32-emscripten support for pyo3. We are able to build the cryptography package:
pyodide/pyodide#2378
There are still some issues with the toolchain which require patching:
llvm/llvm-project#55786

Another issue that I have is that for some reason pyo3 produces blah.wasm files rather than blah.so files. In our cargo/rustc wrapper, we look for .wasm files and move them to .so files. This would probably be an easy fix but I don't know where the problem comes from. We also use -Zbuild-std so we need to use nightly.
https://github.com/pyodide/pyodide/pull/2378/files#diff-dcdc2b45d6538a6db4fa7c76a3b29c56f5f7e7669ab7460385c4241d6d453ce2
If these are things that could be improved in PyO3 or if I am just missing some setting, it would be helpful to know.

I see on #1522 you said:

If anyone with knowledge of how to build a Python interpreter for wasm wants to contribute a full CI job, that would be cool!

I would be interested in contributing a CI job for wasm32-emscripten. Options:

  • build against 3.11b1 -- we wouldn't need to patch the Python interpreter, but this may require the rest of the PyO3 dependencies to work against 3.11b1 which it might not be ready for
  • build against 3.10 -- Pyodide already does this. We would need a few patches, but probably some of our patches can be dropped.

Also, until wasm-ld is fixed to not be mad about lib.rmeta files we will need a patch for emscripten. (I'm actually a bit surprised you don't run into this problem with wasm32-wasi.)

Build time for the wasm32-emscripten Python interpreter is generally a bit shy of 15 minutes. I think we could run the tests themselves in node.

@messense
Copy link
Member

We are actively testing Python 3.11 development versions on CI so I think it should be ready.

"3.11-dev",

@davidhewitt
Copy link
Member

Hey, thanks for offering to help us build up wasm support! With the Python 3.11 wasm interest, it feels like a great time to start testing this in CI. I agree with @messense that adding a new CI job using the Python 3.11 beta is probably the easier choice? Honestly though I think you're the expert in this domain, so happy to take your lead on whichever seems simpler.

Let me know if you need any pointers in order to add the CI job?

Another issue that I have is that for some reason pyo3 produces blah.wasm files rather than blah.so files. In our cargo/rustc wrapper, we look for .wasm files and move them to .so files.

I suspect that that's fundamental to either rustc or cargo? We don't actively control the name of the output in PyO3 itself. This sounds like something we could put into setuptools-rust and/or maturin if you're building via those.

We also use -Zbuild-std so we need to use nightly.

Oh, I'm curious why? rustup component list does suggest rust-std-wasm32-unknown-emscripten is available for download, there presumably is a reason that's no good?

(I'm actually a bit surprised you don't run into this problem with wasm32-wasi.)

We only run cargo check with wasm32-wasi, so it's quite possible we do but nobody told us. My wasm knowledge is limited and it's been beyond my availability to investigate all the wasm targets; there's probably more that can be done to improve the -wasi target too.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 4, 2022

Python3.11.0b1 it is then.

Let me know if you need any pointers in order to add the CI job?

Will do, still working on getting the tests working in an ad-hoc way on my local setup. They mostly work but cargo test doesn't know how to run the emitted .js file, I have to run in manually. Is there a way to tell cargo test to run the test via node test.js rather than via ./test.js?

I suspect that that's fundamental to either rustc or cargo?

You can print out the target info with:

RUSTC_BOOTSTRAP=1 rustc -Z unstable-options --print target-spec-json  --target=wasm32-unknown-emscripten

and you see: "dll-suffix": ".wasm". We wish we could have "dll-suffix": ".so" but I think you would need to make a custom target because there is no indication that this is otherwise configurable. See:
https://doc.rust-lang.org/std/env/consts/constant.DLL_SUFFIX.html

This sounds like something we could put into setuptools-rust and/or maturin if you're building via those.

Yeah that would probably be useful.

We also use -Zbuild-std so we need to use nightly.

Oh, I'm curious why?

Well I tried removing it and... it works fine without it. So we don't need nightly =)

@messense
Copy link
Member

messense commented Jun 4, 2022

Is there a way to tell cargo test to run the test via node test.js rather than via ./test.js?

Custom cargo runner should do: https://doc.rust-lang.org/cargo/reference/config.html#targettriplerunner

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 4, 2022

Okay, on my local setup, all tests pass except the following:

  1. test_date_fromtimestamp in the unit tests has a function signature mismatch. This probably requires fixing in pyo3. This is a segfault so later unit tests don't run.
  2. panic_unsendable_base and panic_unsendable_child fail in test_class_basics.rs
  3. test_compile_errors fails with "ERROR: failed to execute cargo: Function not implemented (os error 52)" which looks like a clear xfail
  4. test_dict_iter fails iter_dict_nosegv: thread 'main' panicked at 'Failed to set_item on dict: PyErr { type: <class 'MemoryError'>, value: MemoryError(), traceback: None }', /src/wasm/pyo3/src/types/dict.rs:396:18
  5. test_filenotfounderror fails
  6. test_proto_methods.rs fails I think in the contains test, it looks to me like an Emscripten bug most likely

@davidhewitt
Copy link
Member

Nice! We can always start by adding #[cfg_attr(target_arch = wasm32, ignore)] on problematic tests to get something merged and then pick at the remainders. (I might not have that target_arch invocation exactly right, on my phone atm.)

  1. Hmm can you elaborate on the signature mismatch?
  2. I think panics probably don't work on wasm, probably just ignore those for now.
  3. Yeah we can live without UI tests on wasm.
  4. Potentially the dict allocated by the test is too big for wasm? We can reduce the size and see if that fixes; the test takes a long time anyway and probably doesn't need to.
  5. I don't know much about various wasm abis and filesystems they provide, probably not worth trying to fix for now.
  6. Happy to take a look at whatever output you have to see if I can offer any insight.

@davidhewitt
Copy link
Member

davidhewitt commented Jun 8, 2022

Will keep this open until we solve the tests with remaining issues.

@davidhewitt davidhewitt reopened this Jun 8, 2022
@messense
Copy link
Member

messense commented Jun 8, 2022

I'm also interested in adding wasm python extension module build support to maturin and setuptools-rust, I'd love to learn more about how to implement it, for example how to build a python.wasm file and run it in nodejs or wasmtime.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 8, 2022

I am looking into adding emscripten-wasm CI support for setuptools-rust next.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 8, 2022

Also, very happy to provide advice about building for the emscripten-wasm target. I don't know as much about wasi, and I've been putting off on learning it because it doesn't support dynamic linking yet.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 8, 2022

Actually, it seems like all of pyo3 requires libffi? The only wasm target that currently has a libffi port is emscripten-wasm32 so I don't think you will be able to build for wasi-wasm or unknown-wasm until that is fixed.

Though it's not really clear that it's possible to make a libffi port for wasi-wasm since wasi doesn't have a way to make flexible argument call trampolines. I guess if you don't need closure support you could make a predefined list of trampolines based on what signatures you think you might need at runtime and just fail if someone tries to make a libffi call outside the list.

@davidhewitt
Copy link
Member

Actually, it seems like all of pyo3 requires libffi

That's interesting, can you detail to me why this appears to be the case? We don't knowingly link to libffi, unless this is a core piece of upstream Rust?

@hoodmane
Copy link
Contributor Author

hoodmane commented Jun 9, 2022

That's interesting, can you detail to me why this appears to be the case?

In that case I'm probably wrong! I think I read examples/rust_with_cffi/setup.py and thought it was for all setuptools-rust projects.

@hoodmane
Copy link
Contributor Author

Okay now I remember why I was using -Z build-std. Sometimes I get errors like follows:

wasm-ld: error: /src/.docker_home/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/\
lib/rustlib/wasm32-unknown-emscripten/lib/libstd-e14e219672153ff7.rlib(std-e14e219672153ff7.std.49da7a3a-cgu.0.rcgu.o): 
relocation R_WASM_MEMORY_ADDR_SLEB cannot be used against symbol `.Lanon.723cf3bb54e6bddb0910b844743e191c.34`; 
recompile with -fPIC

I'm not sure why I'm not getting them when building the cryptography package anymore.

@samuelcolvin
Copy link
Contributor

This is really exciting, thanks everyone for your work on this.

I'd love pydantic-core to be a guinea pig for releasing packages with wasm support. It should be a good candidate as there's no disk or network IO involved.

I think we're pretty much ready an initial release of pydantic-core, is there anything I can do to encourage this? Or somewhere else I should submit a PR or issue?

@messense
Copy link
Member

messense commented Jun 21, 2022

@samuelcolvin Emscripten support was added to maturin in PyO3/maturin#974 which was released in v0.13.0-beta.7, you can install it from PyPI using pip install -U --pre maturin.

To build a wheel, with emscripten installed, run the following command with a Rust nightly toolchain

maturin build --manifest-path <Path to Cargo.toml> --target wasm32-unknown-emscripten -i python3.10

@samuelcolvin
Copy link
Contributor

thanks @messense pydantic/pydantic-core#106.

@davidhewitt
Copy link
Member

Is there anything remaining in this issue? We have emscripten CI in all of PyO3 / maturin / setuptools-rust which appear to be working, so I think this can be called resolved?

@samuelcolvin
Copy link
Contributor

Agreed.

@hoodmane
Copy link
Contributor Author

hoodmane commented Aug 20, 2022

Well there's a couple more things I'm interested in:

  1. I'd like to run the tests that mix Rust and C/C++. This is quite common in real packages.
  2. I'm working on a way to have Pyodide virtual environments Add command to create Pyodide virtual environment pyodide/pyodide#2976 this will lead to rather more graceful testing.

It would be reasonable to close this one and open one or more new issues for these more specific things.

@davidhewitt
Copy link
Member

Sounds good to me; will track follow-ups in #2582.

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 a pull request may close this issue.

4 participants