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

Problem with nightly and rustflags "+atomics" #6

Closed
mhchia opened this issue Sep 2, 2023 · 6 comments
Closed

Problem with nightly and rustflags "+atomics" #6

mhchia opened this issue Sep 2, 2023 · 6 comments

Comments

@mhchia
Copy link
Contributor

mhchia commented Sep 2, 2023

(Updated the instructions. Big thanks to @themighty1 for reproducing and finding the issues in these steps.)

What's wrong?

To bring back rayon, we need wasm-bindgen-rayon. It requires us to use nightly rust with rustflags for "+atomics,+bulk-memory,+mutable-globals"

Steps to Reproduce

Step 1: Open a new terminal. Build and run a websocket proxy

git clone https://github.com/novnc/websockify && cd websockify
./docker/build.sh
docker run -it --rm -p 55688:80 novnc/websockify 80 twitter.com:443

Step 2: Open a new terminal. Run a modified notary server in twitter example

git clone [email protected]:mhchia/tlsn.git
cd tlsn && git checkout tlsn-examples-ws
cd tlsn/examples
RUST_LOG=debug,yamux=info cargo run --release --example notary

Step 3: Clone the branch mhchia/debugging-atomics-issue from tlsnotary/tlsn-extension. Using this branch to debug so that we don't need to load the extension.

git clone [email protected]:tlsnotary/tlsn-extension.git
cd tlsn-extension
git checkout mhchia/debugging-atomics-issue

Step 4: Use nightly rust by creating a file rust-toolchain with the content nightly-2022-12-12 under tlsn-extension project root.

echo "nightly-2022-12-12" > rust-toolchain

Step 5: Add a file .cargo/config under tlsn-extension project root with the following content, as wasm-bindgen-rayon suggested.

[target.wasm32-unknown-unknown]
rustflags = ["-C", "target-feature=+atomics,+bulk-memory,+mutable-globals"]

[unstable]
build-std = ["panic_abort", "std"]

Step 6: Build the wasm and run the dev server

yarn build-and-start

Step 7: Open the browser (I've been using Chrome), visit http://localhost:8080/, and open the console. You should see

image

and some logs, and then

image

You shouldn't be able to see !@# 3 in the console.

@mhchia mhchia changed the title Problem encountered when using rustflag "+atomics" Problem with nightly and rustflags "+atomics" Sep 5, 2023
@mhchia
Copy link
Contributor Author

mhchia commented Sep 5, 2023

Seems to do with websocket. 🤔 With nightly + rustflags, writing to a websocket results in an error.

image

@mhchia
Copy link
Contributor Author

mhchia commented Sep 6, 2023

With nightly + rustflags, reading and writing to the websocket results in an error.

Steps to reproduce

Step 0: Run notary and websockify as #6 (comment)

Step 1: For extension, checkout to the branch mhchia/debugging-nightly-atomics-websocket

git fetch origin
git checkout mhchia/debugging-nightly-atomics-websocket

Step 2: Ensure .cargo/config has the content

[target.wasm32-unknown-unknown]
rustflags = ["-C", "target-feature=+atomics,+bulk-memory,+mutable-globals"]

[unstable]
build-std = ["panic_abort", "std"]

Step 3: Build and run. You should see "Error: Failed to write to websocket" in the console

yarn build-and-start

image

It works without rustflags

Step 4. Rename .cargo/config so that it's not used when compiling. Build and run again. Wasm is running correctly and you shouldn't see the error "Error: Failed to write to websocket"

mv .cargo/config .cargo/config.bk
yarn build-and-start

image

@mhchia
Copy link
Contributor Author

mhchia commented Sep 6, 2023

I've made a smaller example to reproduce: https://github.com/mhchia/test-ws-stream-wasm. Also, reported in najamelan/ws_stream_wasm#12

mhchia added a commit to tlsnotary/ws_stream_wasm that referenced this issue Sep 7, 2023
@mhchia
Copy link
Contributor Author

mhchia commented Sep 11, 2023

Worked around by increasing wasm memory to 4GB

@mhchia
Copy link
Contributor Author

mhchia commented Sep 11, 2023

Websocket error workaround

Shout out again to @themighty1 for locating the issue in ws-stream-wasm and having a patch. With the patch, tlsn-extension can run without websocket errors, until prover is finalizing. Then, I encountered an error which looks an out-of-memory issue.

image

I workaround it by increasing wasm memory to 4GB (4x the default value), and prover can work successfully.

Rayon

By adding back rayon and wasm-bindgen-rayon, tlsn-extension can run with multi-threads.

image

With 6~12 threads on my laptop (hardwareConcurrency=12) , it takes ~2 mins, 3x faster than single-threaded (6 mins).

image

Steps to reproduce

Step 0: Run notary and websockify as #6 (comment)

Step 1: For extension, checkout to the branch mhchia/workaround-ws-stream-wasm-and-work-with-rayon

git fetch origin
git checkout mhchia/workaround-ws-stream-wasm-and-work-with-rayon

Step 2: Ensure you're using nightly rust

echo "nightly-2022-12-12" > rust-toolchain

Step 3: Change wasm memory to 4GB. Copy the following content and paste to .cargo/config

[target.wasm32-unknown-unknown]
rustflags = [
    "-C",
    "target-feature=+atomics,+bulk-memory,+mutable-globals",
    "-C",
    "link-arg=--max-memory=4294967296"
]

[unstable]
build-std = ["panic_abort", "std"]

Step 4: Build and run

yarn build-and-start

You might see some warnings but it's fine

image

Step 5: Open the browser (I've been using Chrome), visit http://localhost:8080/, and open the console. It should run successfully.

Concerns

This workaround only unblocks us to test if rayon can work in browser. Should try to find a better solution without increasing the memory, for example, a patch without oom happening.

@mhchia
Copy link
Contributor Author

mhchia commented Oct 3, 2023

Now extension works with nightly and atomics. OOM issue is tracked in #13 .

@mhchia mhchia closed this as completed Oct 3, 2023
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

No branches or pull requests

1 participant