Replace `rand` crate with `fastrand` #32

Gordon-F · 2023-09-26T20:30:22Z

These changes allow piccolo to compile for wasm32-unknown-unknown target.

kyren · 2023-09-27T10:22:59Z

There's no reason to swap to the fastrand crate just to get wasm32 support, it works just as easily with the rand crate and the getrandom dependency. In fact, no changes to piccolo are required at all, except that a downstream project might need to add a false dependency on getrandom with the js feature to make it work. I use piccolo in a browser right now and that's what I do.

I'm not sure what is the most correct thing for piccolo to do though, if it's more correct to enable the getrandom js feature here for the wasm32-unknown-unknown target or let the final application do it.

Gordon-F · 2023-09-28T14:08:24Z

add a false dependency on getrandom with the js feature to make it work

Nice! Sorry, I misread rand docs. I pushed updated pull request with getrandom dependency for wasm32 targets.

kyren · 2023-09-30T00:34:49Z

Yeah, that's more like what I was thinking, but now the question is... is this the correct thing to do, or should we just let the end user do it?

Do we enable the "js" feature unconditionally when the target arch is wasm32? If so, that just kind of raises the question of why doesn't getrandom do this already?

My guess is that the answer is because the target is wasm32-unknown-unknown, javascript isn't really strictly implied.

So, if we accept that, we can EITHER have our own "js" feature which seems pretty heavyweight, or maybe just... leave the status quo and let the end user manually inject the "js" feature into getrandom? Or do something else where we ask the user to seed the global random number generator?

I dunno, that's kind of why things were left like they are now actually, because I didn't know the correct answer.

Gordon-F · 2023-09-30T17:57:28Z

From getrandom doc:

This crate fully supports the wasm32-wasi and wasm32-unknown-emscripten targets. However, the wasm32-unknown-unknown target (i.e. the target used by wasm-pack) is not automatically supported since, from the target name alone, we cannot deduce which JavaScript interface is in use (or if JavaScript is available at all).

About js feature:

This feature should only be enabled for binary, test, or benchmark crates. Library crates should generally not enable this feature, leaving such a decision to users of their library. Also, libraries should not introduce their own js features just to enable getrandom’s js feature. This feature has no effect on targets other than wasm32-unknown-unknown.

daxpedda · 2024-01-03T08:08:37Z

Just my 2¢:

If the only wasm32-unknown-unknown platform Piccolo supports is Web, then I would argue enabling getrandoms js feature when on this target makes sense.

But if Piccolo intends to support other platforms, e.g. running on Wasmtime (requires an API to supply a random function by the user), it should not be enabled and left to the user instead. Though I don't see anything particularly wrong with re-exposing a js crate feature in Piccolo either.

CryZe · 2024-01-03T08:44:44Z

I definitely would want to run piccolo on wasmtime. I have a wasm based plugin system, so running Lua scripts via piccolo would be great.

daxpedda · 2024-01-03T08:59:36Z

(Wasmtime does support wasm32-wasi, so my comment only pertains to running wasm32-unknown-unknown on Wasmtime, which might be a requirements for some use-cases)

kyren · 2024-01-03T12:49:38Z

I'm planning on addressing this in the near future when I add explicit support for no_std. There should definitely be a version that just accepts a random seed from the user.

I think at that point just having a "getrandom" feature would be enough, and that can provide a math lib load function that doesn't require a random seed.

We could also provide maybe both "getrandom" and "getrandom-js" features so the user doesn't have to manually inject the "js" feature, but this is quite a lot of ceremony for what amounts to a single function call.

Gordon-F force-pushed the fastrand branch from 1d3c2b2 to 4d5ceb1 Compare September 26, 2023 20:37

Add getrandom dependency for wasm32 target

a415caf

Gordon-F force-pushed the fastrand branch from 4d5ceb1 to a415caf Compare September 28, 2023 14:02

kyren force-pushed the master branch 2 times, most recently from 194308a to b0e9412 Compare November 22, 2023 05:43

kyren force-pushed the master branch from 77630cb to fcbaabc Compare August 26, 2024 02:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `rand` crate with `fastrand` #32

Replace `rand` crate with `fastrand` #32

Replace rand crate with fastrand #32

Are you sure you want to change the base?

Replace rand crate with fastrand #32

Conversation

Replace `rand` crate with `fastrand` #32

Replace `rand` crate with `fastrand` #32