On Wed Dec 7 20:46:32 2022 +0000, Gabriel Ivăncescu wrote:
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. :)
I meant something like cycle collector participant. Note that only one per object type is needed, not one per object instance. In practice, when annotating a reference, when we know the type of referenced thing, we may pass that information to CC/GC. It doesn't even need to be accessible from object itself (although it could be convenient and having IUnknown for scope chain doesn't sound too bad).
Since we take care of not holding cyclic refs already in mshtml builtins (non-JS objects)
We have some cycles, that's how nsIDOMNode <-> HTMLNode works, for example. We depend on cycle collector to handle it.
What I mean is, we can't mix a mshtml object's refs with jscript one to create cyclic refs between them
Of course we can, here is an example:
``` var div = document.createElement("div"), div2 = document.createElement("div"); div.appendChild(div2); div2.prop = { d: div }; ```
The whole point of integrating it is to handle such cases. We already have CC integration, so something like following example will be collected (and potentially broken by your proxies; I didn't look at recent version, but I recall pointing it out in the very early version of your patches):
``` var div = document.createElement("div"), div2 = document.createElement("div"); div.appendChild(div2); div2.prop = div; ```
But to fix the first example, GC/CC needs to be able to handle all kinds of objects: JS, MSHTML and Gecko.