Feature-complete RR

[arjunr2]

Notes

• Support for all entrypoints through Func, TypedFunc, and ComponentFunc
• Support for all component builtins
• Support for all instantiation entrypoints
• Support for both sync and async
• Fully embedding agnostic replay driver

Structure

• All core capabilities are feature-gated with the rr feature. The only things exposed without the feature are rr::hooks, which offers convenient methods to hook recording/replaying mechanisms into the rest of the engine.
  As a result, rr without hooks should be able to land with no impact on existing Wasmtime whatsoever.

Observed overheads

Between 4-8%

[cfallin]

This is generally pretty good -- thanks a bunch for all of your hard work here!

I have a ton of nits below but I don't think there is anything fundamentally wrong with any of the design. This is more a lot of polish and getting details right.

My eyes did glaze over at several parts around macro usage, and innards of the component ABI. Definitely will need careful review by Alex and/or others when this lands for real. But this should be a start at least. Happy to discuss any of the below.

[cfallin]

Rather than AsRef<[u8]>, which I think means that the reader must materialize the rest of the trace into memory, could we have a separate fn read(&mut self, len: usize) -> &[u8] that reads len bytes and returns a slice? If EOF then the slice may be shorter than len. This would allow an implementation with a buffer fed by underlying storage on demand (e.g. from a file).

[arjunr2]

I thought with no_std, the point is that you don't have a file-like storage? Isn't the implication that everything is materialized in memory?

[cfallin]

Well, no, the literal implication is that there isn't a standard library. The user's custom environment may very well still have storage abstractions. Consider e.g. dropping Wasmtime into a big embedded system with a custom OS, or something like that.

[cfallin]

Thanks for the quick response! I skimmed over most of the new commits; the one that I want to understand more closely (with non late-night brain) is the change to permit reentrancy (522bb60). I'll take a look at that one separately in a bit.

[arjunr2]

Btw, I have a commented out line of code in the repo now to throw a compile error if component-model-async and rr are enabled together (currently disabled since CI needs to pass). We should figure out how to add that into the CI as well

[cfallin]

Btw, I have a commented out line of code in the repo now to throw a compile error if component-model-async and rr are enabled together (currently disabled since CI needs to pass). We should figure out how to add that into the CI as well

Hmm, looking through the CI config, it's more than one cargo check (the one I would expect is on line 373: checks -p wasmtime --all-features). It's e.g. the docs build as well, and probably other things that assume that one can turn on all features.

Perhaps we can put something in Store::run_concurrent (the main entrypoint for actually running async component model code) that errors out if RR is enabled on the store?

[cfallin]

I left one comment in the reentrancy commit re: lifetimes (can we try harder to avoid the unsafe, basically; happy to help think through this). More generally though I'm not sure I understand the nested event loop thing in the return hook -- I mean, I understand how architecturally one could be forced into it, but ideally I'd want replay to be one loop at one level -- that also makes eventual trace seeking (which we'll need for reversible debugging) feasible, versus having to have a nested series of stack frames in certain states.

[arjunr2]

Yeah, this is challenging, I thought quite a bit on how to get re-entrancy to work and had settled on this.

The unsafe doesn't really have to do with lifetimes. It's that to accomplish re-entrancy, the replay stub for the host functions need access to the ReplayInstance which created it. The way I accomplish this right now is tying a special ReplayHostContext to the Store, that is fully opaque to users and only creatable by the ReplayInstance. This context allows us to access instance state and run the appropriate re-entrancy call. So basically, if we replay, we can assume the data has to be a ReplayHostContext, which is what the unsafe is doing. The other option to do this is to potentially have a field in the Store that has a weak pointer to a ReplayInstance, and access things through that. Maybe this works better?

As for replay being one loop at one level, is it possible to do that? I'm not sure how that's even possible with re-entrancy, because there is no getting around actually invoking the wasm function from the return hook right? The "loop" in the return hook is just because we can have re-entrancy arbitrary levels deep. The only thing it expects is a balanced set of entry/return events.

[cfallin]

FYI, mentioned this PR to Alex today in the Wasmtime biweekly and he will take a look sometime in the next few weeks -- cc @alexcrichton.

[cfallin]

As for replay being one loop at one level, is it possible to do that? I'm not sure how that's even possible with re-entrancy, because there is no getting around actually invoking the wasm function from the return hook right? The "loop" in the return hook is just because we can have re-entrancy arbitrary levels deep. The only thing it expects is a balanced set of entry/return events.

Thinking more about this, it seems like a very core challenge for reversible debugging. We fundamentally need to snapshot and restore to earlier execution states. Our plan-of-record for the active stack frames has been to actually memcpy out the fiber stack (the active parts of it, down to saved SP) and copy it back in; if no pointers move, then everything is still valid.

But if we have native frames between Wasm activations, all that goes right out. There's no safe way at all to restore a set of native frames that core Wasm called out to, that called back into Wasm.

Maybe we don't support reentrancy during record or replay; maybe the component model got this right. (Side-eyeing upcoming changes to support reentrancy that I've heard rumblings about -- I don't know any details.) In this case, we'll want to (I think) trap on attempted re-entry into Wasm when recording...

[arjunr2]

Actually yeah, I just realized, it's more than native frames, we'd actually need native snapshots between activations (which ends up being just like RR!) since Store/Instance state can be modified too.