wren icon indicating copy to clipboard operation
wren copied to clipboard

Introduce RawValue abstraction

Open erincandescent opened this issue 2 years ago • 7 comments

This MR introduces a "WrenRawValue" abstraction. This is a tricky API, so this PR is in part for discussion.

Firstly, this MR incorporates a rebased version of the changes from #911. Since the new trace hook is called on every garbage collection, this is pretty much necessary to not have horrendous performance issues.

Currently the internal Value representation (NaN boxing or not) is currently not exposed as a part of the ABI, and I wanted to preserve that. Therefore WrenRawValue is defined as a simple union:

typedef union WrenRawValue {
  uint64_t  bits;
  void     *ptr;
} WrenRawValue;

When NaN boxing is use, we just stuff the NaN boxed value into the bits member. When NaN boxing is not in use, we just stuff Object pointers directly into ptr. For other value types (Bool, num, etc), we box them by constructing a (newly introduced) ObjIValue to wrap them. The handling of these boxes is entirely contained within the RawValue logic, so the rest of the code need not be concerned.

This MR of course also adds functions to move values between stack slots and WrenRawValues, and one to initialize a RawValue to NULL.

It also adds a trace callback to foreign classes, which is how the GC becomes aware of RawValues. This is the tricky part of the API: it is users' responsibility to properly trace all of their RawValues.

Things I'm especially interested in commentary on:

  • Is this a direction we want to go? (This kind of interface is definitely something I want for my usage of Wren)
  • Naming (RawValue and ObjIValue), since neither of them make me particularly happy
  • General thoughts on the implementation.

--

One other thing I have considered is an alternate (or even complementary) implementation of this concept where the values are guaranteed pointer sized. This is useful when encapsulating references in C APIs which give a "void *userdata" pointer or similar. This would have a similar implementation, but would necessitate the use of ObjIValue boxing on 32-bit platforms even with NaN tagging. Ideally I think I'd have both for efficiency reasons, but I realize having two complementary ways of doing the same thing (even in the 'advanced APIs' section!) could be confusing

erincandescent avatar Feb 19 '22 12:02 erincandescent

By the way: one long term direction I see coming out of adding these trace hooks is that it would allow List, Map, etc to be less special cased inside the interpreter - they could just be another foreign class, and provide a trace hook like those do, even though they'd use the internal Value representation directly instead of the public RawValue abstraction.

erincandescent avatar Feb 19 '22 12:02 erincandescent

The idea is interesting, though I found the change set to be not atomic enough. Maybe a better name would be "WrenWeakValue" instead of "WrenRawValue".

There is at least the obligatory typo ("internAAl") and maybe others.

mhermier avatar Feb 19 '22 15:02 mhermier

I'm most curious on the use case here like what do you want those bits for? in particular since we have WrenHandle it seems to overlap with, the use case is unclear atm.

ruby0x1 avatar Feb 19 '22 20:02 ruby0x1

WrenHandle roots the target object - preventing it from being garbage collected. This does not - it only keeps the target object alive as long as some object is tracing it through the trace hook.

Effectively this allows foreign classes to hold references to other objects without issues such as causing uncollectible reference cycles

erincandescent avatar Feb 19 '22 21:02 erincandescent

As for WrenWeakValue - weak values in other languages automatically set themselves to NULL if their target is freed; this isn't the case here (any RawValue you don't trace will be left dangling)

I don't think RawValue is a good name, but I did want to convey 'this is a somewhat advanced API which needs to be used with care'

erincandescent avatar Feb 19 '22 21:02 erincandescent

I did a bit of digging and the analogous feature in v8 is called v8::TracedReference. SpiderMonkey has an equivalent, but I couldn't find the documentation. JavaScriptCore has JSManagedValue with the same purpose in its Objective-C API, but the objective-c wrapper managed tracing for you.

I lean towards calling this WrenTracedValue; I think the v8 terminology is good there. The documentation would have to emphasise (as does the v8 documentation) the necessity of actually tracing such a value to ensure its validity. one of WrenTracedReference or WrenTracedPointer can be reserved for a hypothetical future guaranteed-void*-sized variant.

I'm thinking of adding an example which implements a fixed-size array using the new APIs, to demonstrate their utility

erincandescent avatar Feb 21 '22 12:02 erincandescent

While I agree this solution is elegant, there is maybe another solution that would bring more to some in the long run. Allowing Foreign to have data members. In practice it means to fusion in a unique struct the following ones: https://github.com/wren-lang/wren/blob/4ffe2ed38b238ff410e70654cbe38883f7533d3f/src/vm/wren_value.h#L418-L428 And than, with the dedicated API around it, we can access all values in any forms and not only pointers to Obj.

mhermier avatar Feb 21 '22 12:02 mhermier