-
Notifications
You must be signed in to change notification settings - Fork 260
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
Add example for wasm_threads usage #841
Conversation
Thanks for the PR. That's awesome. I think we already have something for the headers on main. I'll try to document that. So we could add this too. |
Using [serve]
headers = { "X-Foo" = "bar" } |
@ctron If you've got something else let me know. I think this is about as well documented as it can be, I hope to avoid issues being created about this (if you look at And thank you for the hint about the .cargo/config.toml file working for this at all, and being respected by trunk. IDK if I had found this otherwise. |
Cool, thanks. One think that's missing, sorry for overlooking that before, is to add it to the list of CI example tests: trunk/.github/workflows/ci.yaml Lines 164 to 180 in f65cc10
|
right. that would've been to easy. The pipeline doesnt have nightly. My pipeline uses I won't be able to get to this until tomorrow evening at the soonest. |
No worries, take your time. I think it's close! |
Hmm the failure wasnt because no nightly was present, but because it just used stable. In other words, it ignored the toolchain file. The
In any case where we run this in the CI, we will probably have to further adjust the CI to download the correct toolchain (nightly with some additional components for this example, perhaps other modifications like additional modules for future examples). So far |
So having a So I think the problem is with 1.) … that it's not respecting the base directory of the config file. |
Looks like that's more a cargo issue rust-lang/cargo#11036 (comment) … however, I think we need to fix it. |
Yeah, you seem to be right, looking closer at the logs this seems to be what's happening.
Okay, that would make sense if trunk invokes it like that. So about that fix. Still, 3 ways.
How often do you expect that |
I think I've a PR for this: #844 |
I merged this PR. So maybe you can just rebase and we see if this does the trick. |
Nice, so we were able to solve this ourselves. It worked locally, so I think this should be it ;-P. |
Thanks for finishing this up. That's a great example! |
Thanks for working on this! I've just tried the new example and it works... mostly. (The same too much recursion error also appear with my more complex app, but it could be a me issue) Console:
Any ideas what's going on? |
I've seen this too at times. It went away reliably in release mode though. And since I've used this in my App which always compiles dependencies with optimizations and it works fine. I'm not entirely sure how it comes to be, but I don't think that's got anything to do with trunk. |
Yes, compiling in release mode fixes it, weird. |
It going away with optimizations is probably coming from something like tail recursion optimization. I'm mostly surprised where that amount of recursion is coming from. But I can't really see this being caused by trunk. It going away with optimizations on rust code means its gotta be in rust code - unless IÄm missing something -, and AFAIK trunk doesn't add a lot, or nothing, of that to the runtime of the website, and that optimization setting doesn't affect trunk either. I'm also not entirely sure whats the result of that error either. So if you add this to the Cargo.toml it also goes away: [profile.dev.package."*"]
opt-level = 2 This will make clean rebuilds slower tho. I did not test wasm_thread on its own, to be clear. I did never clone their repo. |
Yes, optimizing dependencies fixed it for me. |
I'll add it and improve grammar in a pr. I'll be terse, as I'm still not certain of whats really going on. |
Would be an example for #680.