-
Notifications
You must be signed in to change notification settings - Fork 31
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
panicked at 'time not implemented on wasm32-unknown-unknown' #145
Comments
lrpar uses elapsed time to know how long time to try recovering from errors, so it does need to have some notion of wall-clock time. I wonder if you can include rust-lang/rust#48564 (comment) in your project? Or whether wasm32-wasi is a suitable target for your usage? |
Could you give some insight into what exactly the time is used for – is it some limit until the computation is deemed too expensive and the error recovery is aborted?
Unfortunately, that won't affect the
That sounds interesting, however multiple parts of the toolchain don't seem to support it yet. Also, if I understood correctly, If it doesn't complicate the code base too much, maybe the timing feature could be guarded behind a cargo feature flag? |
In essence, yes. [It's slightly more complex than that: error recovery is unbounded in time and space. It turns out that using a time limit is a fairly easy way of stopping us using too much memory!]
I think we need some way of specifying time (for the reasons above) . If we can find an unobtrusive way of doing this, I guess it's OK. For example, perhaps we can conditionally include a file along the lines of rust-lang/rust#48564 (comment) then we wouldn't make the rest of the code base any worse, since everything would be isolated to one file. There might be a quick alternative, if you don't need error recovery, that might prevent |
Thank you, adding The time limit seems like a plausible solution regarding computation time (and setting a hard limit for when the user can expect a result). However, one could consume more memory than desired when execution is vastly faster on the target system. I wonder if tracking the number of iterations / recursion depth and limiting the execution accordingly would achieve the same? |
The only possibly better mechanism I can think of is to track memory usage, but that might be hard on some platforms. The current time limit is safe on every grammar I know, and single-threaded performance would probably have to improve by a factor of 4-5x before it became an issue. For better or worse, such an increase in performance seems unlikely to happen any time soon ;) |
Wouldn't tracking some sort of stack depth be a good approximation for that?
True that. Before I head into the adventure of making those changes – what do you think of the following approach: Along the In my case, I then could conveniently provide an equivalent. |
Unfortunately not. [Previous works tried using vaguely similar ideas, and it's easy to construct grammars that cause them to explode.]
I'm reluctant to expose knobs like this to the user because it's going to confuse them unnecessarily: it's got to be One obvious question occurs to me: can we change the Rust WASM backend to call the JavaScript timer? That would solve the problem for grmtools and other libraries too! |
This might be relevant https://crates.io/crates/instant. |
Having though about it, I share your idea that this issue should be resolved on a platform/backend level rather than in the library. Implementing syscalls for Wasm doesn't seem to get too much traction, though: https://internals.rust-lang.org/t/what-is-the-plan-regarding-libstd-and-wasm-syscalls/8497. For my needs I think I'll just fall back to patching the grmtools dependency in my project. I'll close this issue for now since I don't have any actionable input moving forward. I'll keep an eye on how the Wasm / Wasi / syscalls story progresses.
This would be an alternative if you want to add support for Wasm directly to this library and consider adding that dependency to be elegant enough. |
Hey there,
as already mentioned in #126 I had some problems on target
wasm32-unknown-unknown
with gracefully handling input that isn't accepted by the grammar.I got stack traces working now:
See rust-lang/rust#48564 for more info on that issue in general.
Having a quick glance at the code base, it looks like the calls to
std::time::Instant::now
could be replaced by calling a function that produces strictly incremental identifiers.Is that correct, or does some part rely on the actual timestamps?
The text was updated successfully, but these errors were encountered: