On Wed Dec 7 18:09:41 2022 +0000, Jacek Caban wrote:
No, that's not what I meant. I hope that we will be able to re-use traversal code for both standalone and MSHTML GCs.
Okay, I skimmed a bit through the GC in gecko (`xpcom/base/nsCycleCollector.cpp`). I don't know if I got it right, but at a high level, it seems to collect them by allocating `PtrInfo`s for each participant ("object") when collecting. It does this for a long time, lifetime of object even (if it's colored "black"), so not just during the GC traversal itself. It seems to use 2 pointers, two 32-bit values, and a mFirstChild iterator (I assume it's pointer-sized?) for each PtrInfo, so that's quite large overhead.
With my approach I store a list of objects in the script ctx, so only 2 pointer sized extra per object (for the list entry). The gc_marked flag is free due to padding (and anyway if it wasn't I could make it a bit field). Considering objects are way more common than scopes, this seems like the overhead from the hack of having scopes be objects isn't so bad, since it's offset by the GC storing way less information per object.
Well for now I go with my approach since it's far simpler to reason about, but I'm curious about this, since I'm still not entirely sure if the way gecko does it is necessarily an improvement, at least for wine's jscript. Gecko's does work with arbitrary objects, though.
So the potential advantage I can see from gecko side: it works for all participants. But I wonder if that's really useful for us and the jscript in particular we use. Since we take care of not holding cyclic refs already in mshtml builtins (non-JS objects), and only arbitrary jscript code can create them that's out of our control, I don't really see how they can end up being created otherwise? So it doesn't seem like much of an advantage at all.
What I mean is, we can't mix a mshtml object's refs with jscript one to create cyclic refs between them (well, we can with my JS proxies patches but, then they become JS objects, so it still works because the GC can deal with them), so the potential advantage is not really existent.
Only issue I can see, is different script ctx, which can create cyclic refs now due to arbitrary js code doing it between two different script ctx. I think it's not likely in practice, but this could be handled specially though. It's rare enough (maybe doesn't exist at all in the wild) that there's no need to include it in first attempt IMO.
Now towards my understanding, and an idea how to get rid of the hack:
From what I can see, to avoid an "extra vtbl for each participant", it abuses QueryInterface so it returns a static vtbl (i.e. one in which you can't QI back to original object anymore). I mean `IID_nsXPCOMCycleCollectionParticipant`. Am I right?
But setting aside whether that's acceptable in wine or not, the thing is we don't have an interface for `scope_chain_t` at all, so it would still need one.
If it was, do you think I can do something like that "abused" QI (or extend the object's IDispatchEx itself with an extra internal method) to get rid of the jsdisp thing with `scope_chain_t`?
We'd still need a dummy interface (for the QI/extra method) for `scope_chain_t`, though. But at least, the jsdisp would re-use its own.
Sorry for the long winded reply. :)