-
Notifications
You must be signed in to change notification settings - Fork 19
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
Elimination of manual memory management (by default and/or within an init option) via JavaScript's FinalizationRegistry
/WeakRef
?
#115
Comments
I'm curious what @PhoenixIllusion and @LeXXik think of this. I'm using the webidl binder script to generate the bindings between JS and C++ and while it gets you going very quickly, it is quite restricting in what it can do. As far as I know you cannot control how the JS bindings are created, so implementing your suggestion probably means scrapping the whole webidl binder and redoing the entire API from scratch. The emscripten documentation mentions using the FinalizationRegistry but I haven't seen it actually do it. |
I am trying to think of what cases we'd actually want uncontrolled garbage collection. Almost all the critical information of memory references live on the C++ layer, and the JS layer will not have any knowledge of that information. There are a couple classes it could theoretically work (shape/constraint/body settings), but most in general have super-high risks of doing something unintended or wrong. The JS doesn't even really 'own' anything. It just is wrapping a numeric pointer that can trivially be passed round as a u32/u64 and rebuilt inside another light-weight WebIDL wrapper as many times as you want with 100% no impact on the underlying WASM layer, and none of those specific JS wrappers creations or destructions should trigger any WASM destroy calls. Even the task of something like:
WeakRefs would destroy pointerA, even though it is now owned by itemB, because A secondary issue is something where we sometimes randomly do casts that just extract the raw pointers and rewrap them in other JS objects, discarding the first JS class (though webIDL still holds a ref internally, possibly forever):
WebIDL by default does not work well at all for this technique, no. WebIDL will internally hold pointer refs to anything that you ever alloc'd but not free'd in WebIDL (like BoxShapeSettings) using When it's running in production, you will see these pointer storage refs in some extra Dictionary tacked onto the object, like Even if we didn't use WebIDL, I'm just not sure how we could make this work with the current C++ library given how much extra meta-data and control layers you'd need to add to markup every function as 'this method creates a controlled JS object, this one assumes control of it, this one returns a static, non-memory controlled item, this one deletes 3 or more memory-controlled items or an entire pool of cascading deletes on it's own (ex: destroy of a BodyManager)'. Possibly that is approaching the level of writing a custom GC framework on it's own. Possibly if we had a way to extract that data we could capture 80%+ of it through a scrape of the code and some Bison/ANTLR language-parser, but for memory management of freeing, you really can't live with the extra 20 messing up and triggering a double-free and app crash. |
Not sure, if there is much to add, but I personally think it is not viable. At least not today. It works well in Rapier, because |
@LeXXik Do you know if the performance overhead of Embind is practically relevant? This emscripten doc says it's 200ns per call (with the implication that WebIDL is less than this), but that doc was last edited a couple of years ago. If Jolt is making thousands of calls into wasm every frame, then this could have a non-trivial performance impact, but I'm guessing it's a lot less than that? Also seems like work has since been (and is continuing to be) done with Embind performance. Wondering if @RReverser might have any comment on performance implications of switching from |
I haven't measured it, but trust Ammo made an educated decision. Perhaps, @kripken would offer an insight on his experience. However, if you know that an object might not be garbage collected, would you still not manually destroy it? No strong guarantee is just that. Same as no GC, if you ask me. |
Yep definitely interested in kripken's thoughts here!
You're basically saying that the lack of spec-level timing guarantees1 means that GCs are an inherently useless technology? If so, I think the whole JS ecosystem disagrees :) Or are you saying that Embind provides no mechanism at all for explicit freeing of memory? E.g. in case there are unpreventable allocations in a hot path that could do with an explicit free to prevent GC pauses? If that is the case, I guess it also goes in the "is this practically relevant" bucket, since even then the solution tends to just be "find a way to not allocate in hot path". 1 RE the "not at all" from this MDN quote: "When a registered object is reclaimed, any cleanup callbacks for it may be called then, or some time later, or not at all." - I don't think this should be taken to mean anything other than "There is very literally no GC implementation detail that is encoded in the spec / visible to userland." Someone can correct me if I'm wrong here, but I think it's a technical/theoretical point, rather than something that is practically relevant (to devs who understand not to depend on timing -- in e.g. unit tests or whatever). |
One potential path is to split the difference between "recode for auto Gc in different Em framework" and the "webIDL default", by creating a helper library (optional for people to import) along side the main library that takes the commonly used classes that actually need GC and making a factory for them with this wrapper tacked on. It would still be up to a user to know if it's safe to use that version since I'm not sure on edge cases of deleting some shape/constraint settings 100% on weak-ref, given I don't know if Jolt has cases it might assume ownership via a method call. Even EmBind seems to have the same problems without some additional work. I'm still thinking through the gains vs cost of the auto-GC, on this project specifically, when it seems to primarily impact Settings objects and some of the core Math classes (Vec,Quat). Is it mainly just for those classes? The Jolt internal ref-counting is handling quite a bit behind the scenes once you've actually created a constraint/shape/body and passed it into the physics system for ownership. On the math side, personally use 3-4 vectors and maybe 2 quat in my entire library and a heavy use of Vec3.Set and Quat.Set, and so do some of the 3d engines I've seen (a TempVector pattern) |
Trivial Proof-of-Concept of an AutoGC helper class - main...PhoenixIllusion:JoltPhysics.js:feature/auto-gc-tool Heavy console Logging present so I could check the execution paths. I added a WeakRef + Timer fallback which may be a horrible idea. Uses a generic AutoRef method that blindly Jolt.destroy's anything you hand it, and wraps it in transparent-proxy to create a weak-ref-able object with no possibly outside pointers in the core Jolt library, and then passes it to the FinalizationRegistry (or fallback). I added some simple constructor wrappers at the bottom. I've never touched the FinalizationRegistry before so have no clue if this approaches a prod-ready approach or any unknown hazards and cross-browser-support issues. I don't think I'd trust it in my actual job-work on a long-running process unless I had a couple test machines and examples/browser setups and ran a stress-test for a couple hours each. |
Regarding embind/WebIDL binder performance, I believe even today the WebIDL binder is faster, even with recent improvements on embind - embind is just more flexible, and there is a tradeoff there. With that said @brendandahl is working to optimize embind and may have other thoughts here. In particular if embind gains a fully "static" mode where the bindings do no dynamic lookups at all, that seems like it could be as fast as the WebIDL binder (when one uses the same simple types as the WebIDL binder supports). @brendandahl ? |
WebIDL is still be faster today. For functions function exports I think the two can be made the same speed with some improvements to embind. For most workloads though, it seems the overhead of the binding layer is tiny compared to the work that is actually happens in Wasm. I've also seen some projects use embind for most of the API and if there is a really hot code path, use a plain C function export for that. |
I've done something similar in another wasm project and although its not currently merged, it did work quite nicely. IIRC references were tracked wasm side and so the finalization registry allow us to notify that the js side no longer needs the value. Saved a ton of memory bugs and much simpler code for devs |
I'm not sure if this is helpful at all - from latest version release notes:
|
Here are the relevant MDN docs:
My understanding is that using
FinalizationRegistry
, it would look something vaguely like this:So that instead of this:
We can just write this:
If it'd somehow be a non-trivial perf hit or a breaking change, then maybe some sort of init option? (If my opinion is at all useful here, I think it should be default)
I'm currently using Rapier for a project and it's been great in terms of JS idiomaticity (I guess Rust's
wasm-pack
handles memory stuff automatically?), so I'm just posting this in the hopes that Jolt can get to a similarly pleasant DX. Thanks!The text was updated successfully, but these errors were encountered: