[vm/ffi] Allocate structs by default with TypedData

[dcharkes]

When working with a C API that takes structs by value users need to malloc structs in C memory before they can pass them to a C function call, and remember to free the afterwards.

Instead, we should explore changing the API so that the default is that we create structs backed by TypedData rather than allocating them in C memory.

The same argument can be made for Arrays.

TODO: Design an API taking the following into account:

• Users subtype Struct, do they need to create factories/constructors?
• How does the no-argument const default constructor interact with this? We need the consts for annotations. But the non-name constructor would be an ideal candidate for this behavior.

[ds84182]

So a couple of thoughts from my perspective:

1. Structs should automatically generate a default constructor with all field values zeroed.
2. Const constructors would be nice to have, but not required. This could possibly extend to having more types of constants in the future, like zero terminated strings.
3. Unlikely, but struct constructors could, by default, have named arguments to initialize fields. Bonus points if the default value of a struct field can be changed (useful for APIs like Vulkan).
4. Since TypedData structs are still pass-by-reference, a mechanism to copy from one struct to another would be useful when interacting with constants, or other unmodifiable structs.

One question on my mind still is... how will we pass TypedData-backed structs by reference through FFI? Sometimes it would be great if I could just... alloca the struct temporarily on the stack.

[dcharkes]

I think the simplest solution would be to address #53418.

final class MyStruct extends Struct {
  @Int8()
  external int a0;

  external Pointer<Int8> a1;

  external factory MyStruct.fromTypedList(Uint8List typedList);

  factory MyStruct.named({
    required int a0,
    required Pointer<Int8> a1,
  }) {
    final typedList = Uint8List(sizeOf<MyStruct>());
    final result = MyStruct.fromTypedList(typedList);
    result.a0 = a0;
    result.a1 = a1;
    return result;
  }
}

That will require the smallest amount of magic in the Dart SDK.

I'm happy to consider other designs if they serve use cases.

[dcharkes]

Playing around with a possible API more.

Currently, we disallow calling constructors for Struct and Union subtypes all together, so that means a breaking change to constructors is not a breaking change.

Also, the extension method on pointer gives us a type-safe .ref already, so we don't have to make the constructor work with both typed data and pointers.

If we want to design an API only around typed data, we could possibly do the following with some magic in the CFE:

abstract final class NativeType {
  const NativeType();
}

abstract final class _Compound extends NativeType {
  @pragma("vm:entry-point")
  final Object _typedDataBase;

  _Compound._([TypedData? typedList]) : _typedDataBase = typedList!;
}

abstract base class Struct extends _Compound {
  /// Construct a reference to the [typedList].
  ///
  /// Use [StructPointer]'s `.ref` to gain references to native memory backed
  /// structs.
  Struct([TypedData? typedList]) : super._(typedList);
}

final class MyStruct extends Struct {
  MyStruct([TypedData? typedList]);

  @Int8()
  external int a;
}

final class MyStruct2 extends Struct {
  @Int8()
  external int a;
}

void main() {
  // Allocate a struct by allocating a `TypedData` backing the size of the struct:
  MyStruct();
  MyStruct2();

  // Using an existing typed data:
  final typedData = Uint8List(sizeOf<MyStruct>());
  MyStruct(typedData);
}

The CFE will then rewrite the constructors of user-defined structs/unions to:

final class MyStruct extends Struct {
  MyStruct([TypedData? typedList])
      : super(typedList ?? Uint8List(sizeOf<MyStruct>()));

  @Int8()
  external int a;
}

final class MyStruct2 extends Struct {
  MyStruct2() : super(Uint8List(sizeOf<MyStruct>()));

  @Int8()
  external int a;
}

Note that Structs constructor takes an optional argument to allow MyStruct2 to not define a constructor, but because of the CFE rewrites the constructor is always invoked with a non-null value.

I propose we don't add magic for field values but rely on writing factory constructors. (Writing field names types in the arguments would be boilerplate allready, so we might as well do the assignments as boilerplate too in that case and have no magic in the compiler. That is better for understandability.)

final class MyStruct3 extends Struct {
  factory MyStruct3({int? a}) {
    final result = MyStruct3._();
    if (a != null) {
      result.a = a;
    }
    return result;
  }

  factory MyStruct3.required({
    required int a,
  }) {
    final result = MyStruct3._();
    result.a = a;
    return result;
  }

  factory MyStruct3.fromTypedList(TypedData typedList) {
    return MyStruct3._(typedList);
  }

  MyStruct3._([TypedData? typedList]);

  @Int8()
  external int a;
}

If we want to be able to have the no-name factory constructor for users, then we need to take an arbitrary constructor that has an optional TypedData argument as the one to be rewritten by the CFE.

Maybe we should require marking the constructor as external:

final class MyStruct3 extends Struct {
  external MyStruct3.arbitraryName([TypedData? typedList]);
}

The only requirements would be (1) external and (2) the arguments must be exactly [TypedData? typedList]. (Or maybe some freedom in the name typedList.)

Notable design decisions:

• No const. TypedData nor Pointer has const constructors, so we can't have const.
• No named arguments allowed. Use factory constructors to do this.
• All fields are zeroed if no typed-data is passed in (by virtue of typed datas being zero out on allocation).

Edit: We probably also want to generate an assert that checks that typed data byte length is long enough in the constructor rewrite.

[dcharkes]

cc @lrhn @mkustermann Any thoughts about the API?

[lrhn]

So the goal is to allow, but not require, a user written subclass of Struct to be constructed using a provided typed-data for memory.

Such subclasses are currently not constructable. They have a default constructor which cannot be invoked - and are not allowed to declare a (non factory?) constructor.

And we want to add this in a way that does not break existing code, which means that a class with no constructor declaration must remain valid.

That sounds to me like or would be solved by giving Struct a constructor of

  external Struct([TypedData typedData, int offset = 0]);

Any subclass can choose to pass an argument to that constructor. Because the parameter is not nullable, every invocation will be statically detectable as either passing a typed data or not, which might be useful.

And as suggested, calling without a typed data value will allocate one just big enough to hold the size of the current struct.
Passing one will check that

• offset is not negative, and is within the current typed data's range.
• there is room for the current struct type between the offset and end of the typed data.

If so, the struct will refer to the bytes starting at that position.

The offset is there because it can be. If the argument is a TypedData, then it can already point into any position of the underlying ByteBuffer, so adding a further offset (multiplied by size-in-bytes of the provided TypedData) shouldn't make anything more complicated.

Also, it seems useful to be able to do

  var size = sizeOfMyStruct;
  var structs = [for (var i = 0; i + size < bytes.length; I += size)
      MyStruct(bytes, i)];

Instead of having to allocate a view per struct.

This would be a minimal change, and requires absolutely no special casing in the subclasses. They can use, or not use, the ability to pass a byte buffer to the superclass, and prevent users from instantiating by not existing a generative constructor.

It does mean that any existing struct can now be allocated using the default constructor. Which shouldn't be a problem.

The one thing I can't help wondering about is whether allocated structs should be extension types. Everything about this feature sounds like a view on ... something. But a view on "bytes", not an object. On a pointer, it's just that the pointer it's not a Dart object itself, so it doesn't fit extension types directly.

(Also, if we put pointers into typed data to native code, I assume we're sure GC won't move the bytes.)

[dcharkes]

The offset is there because it can be.

We do have views already, so why introduce offset?

The code snippet would be the following:

  final Uint8List typedList;
  final size = sizeOf<MyStruct>();
  final buffer = typedList.buffer;
  var structs = [for (var i = 0; i + size <buffer.length / size; i += 1)
      MyStruct(Uint8List.view(buffer, typedList.offsetInBytes + i * size))];

If we think that's too verbose, shouldn't we think about adding view methods to Uint8List and friends instead?

  final Uint8List bytes;
  final size = sizeOf<MyStruct>();
  final structs = [for (var i = 0; i + size < bytes.length; i += size)
      MyStruct(bytes.view(i))];

The one thing I can't help wondering about is whether allocated structs should be extension types. Everything about this feature sounds like a view on ... something. But a view on "bytes", not an object. On a pointer, it's just that the pointer it's not a Dart object itself, so it doesn't fit extension types directly.

We could theoretically make structs an extension types of TypedData and use Pointer.asTypedList internally everywhere where we want to have struct views on pointers.

Though, wouldn't trying to make structs extension types be a huge breaking change? How would we go about such a thing?

(Also, if we put pointers into typed data to native code, I assume we're sure GC won't move the bytes.)

We would use typed data views, which are updated when the GC moves things.

[mkustermann]

We could theoretically make structs an extension types of TypedData and use Pointer.asTypedList internally everywhere where we want to have struct views on pointers.

Though, wouldn't trying to make structs extension types be a huge breaking change? How would we go about such a thing?

That would be nice, but extension types (aka inline classes) are pure syntactic sugar around an underlying type - the CFE lowers them to the underlying type. But our FFI struct classes can be backed either by a TypedList or by a Pointer and they share no common base class atm (the VM has a concept of PointerBase but the Dart code doesn't)

(Also, if we put pointers into typed data to native code, I assume we're sure GC won't move the bytes.)

We would use typed data views, which are updated when the GC moves things.

The struct instances don't expose their address. One can have a Pointer<FooStruct>, which one has to allocate using malloc /... in C. Once dereferenced to a FooStruct (via pointer.ref) one cannot get the pointer address anymore.

[dcharkes]

That sounds to me like or would be solved by giving Struct a constructor of

  external Struct([TypedData typedData, int offset = 0]);

@lrhn I'm not entirely sure I understand this. We can't have an optional positional non-nullable argument right?

Any subclass can choose to pass an argument to that constructor.

I was thinking to not have users write the super invocation manually, but to have a transformation fill in the details. Invoking the super constructor manually requires the super constructor to be public, but I don't want users to write Struct() or Union(). If the CFE transforms external MyStruct(...) to invoke the private Struct._([TypedData?]) then we don't have to write extra logic to restrict users from calling Struct() or Struct(myTypedData).

We need a CFE transformation anyway for when the user does not write the constructor (because the body needs to be filled in with allocating a typed-data of the right sizeOf), so reducing the amount of 'magic' for when the user does write a constructor would only removes part of the magic.

Did I misunderstand what you proposed @lrhn ?

[lrhn]

We do have views already, so why introduce offset?

I remember someone sayiong that creating a new view is too much overhead. In some context. (Was that you, Martin?)
If you want to create 500 structs, doing:

var fooSize = sizeof<FooStruct>();
var buffer = Uint8List(500 * fooSize);
var structs = [for (var i = 0; i < 500; i++) FooStruct(buffer, i * fooSize)];

seems nicer (and more efficient than) than

var structs = [for (var i = 0; i < 500; i++) FooStruct(Uint8List.sublistView(buffer, i * fooSize)];

@lrhn I'm not entirely sure I understand this. We can't have an optional positional non-nullable argument right?

We can. The parameter just need a default value, which you can't see in an abstract or external function, because that's an implementation detail.

It was deliberate to make the first parameter non-nullable. If you pass a first argument at all, it has to be a buffer, you can't pass a nullable value where the receiver needs to determine whether it was null or not.
Which should help your source transformation, because it can see whether the call is passing a buffer or not.

... But it might make it hard for the user to declare a constructor which forwards either zero or one argument, so it should probably be nullable. I don't know if that makes things harder later.

I was thinking to not have users write the super invocation manually, but to have a transformation fill in the details.

I'm usually in the "favore explicit over implicit" camp.
That has two benefits:

• It allows users to opt out, or customize the constructor to their purpose (instead of having you generate a constructor, and then creating their own factory constructor to wrap it, if that's even possible).
• It makes it clear precisely how the otherwise implicit code is expected to work, because it looks like a normal declaration. You can predict behavior without having to look at a prose description of how the implicit constructor works, one which may forget some particular corner case (like we too often do).

but I don't want users to write Struct() or Union()

If it's an abstract type, they can't.
Then the only code which can invoke the (external) generative constructor is a subclass, and I assume we have strong checks on which declarations are allowed to subclass those types.

Even if it's not an abstract class, it should be "simple" to just prohibit calling the constructors directly, so the compiler complains eagerly if someone does that. I assume most of these classes are heavily special-cased in the VM compiler.

If the CFE transforms external MyStruct(...) to invoke the private Struct._([TypedData?]) ...

then the user can't hold it wrong, but they also can't see what they're supposed to do.
It's black-box coding where you write an external declaration, and the compiler fills in the details. That's OK, but if it's not necessary, I prefer the more explicit declaration.
And again, it would allow the author themselves to control the name. And the signature.

Would the transformation allow:

• external MyStruct.name([TypedData? bytes]);
• external MyStruct._([TypedData? bytes]); -- Not publicly available, I create all instances.
• external MyStruct(TypedData bytes); -- Required typed-data, cannot be heap-allocated.
• external MyStruct(); -- No typed-data, can only be heap-allocated.

and there are things which cannot be done as external

• MyStruct(int offset, TypedData buffer) : super(buffer, offset); -- Swapped parameter order.
• MyStruct([super.buffer, super.offset]) : assert(offset % 48 == 0, "Misaligned!"); -- Initializer lists.
• MyStruct([super.buffer, super.offset]) { this.id = _counter++; } -- Bodies!

We can choose to support all (except the last three), some, or none of these variations with a transformation on an external declaration, but if we can "easily" (I hope) allow all of them by introducing the special casing only at the super-constructor call, not at the subclass constructor code, it just gives much more flexibility. It allows the entire language in those constructors, without us having to allow-list only some particular patterns, and run into users asking for more.

And since the super-constructor can be called with zero arguments, an implicit default-constructor of MyStruct() : super(); doesn't have to be special either (or rather, it's still just the super() which has to be special, everything until that is just a normal constructor. It's just a particularly boring constructor).

So, my thought, which may be naive, would be that the super-invocation, super(buffer, offset), super(buffer) or super(), which the transformation intercepts and modifies, perhaps by pointing it to another secret constructor Struct._(int structSize, int structAlignment, TypedData? buffer, int? offset) to ensure it has all the necessary information.
But it won't fiddle with the user's code, which makes it easier for the user to reason about, and easier for us to not worry about what weird stuff the user is doing before calling into our code.

It would indeed still only remove some magic, but it moves the magic below the user-code, instead of on top of it.
(So many words, no clear distinctive argument. Maybe it's really just a gut feeling that it'll avoid complications or problems down the line.)

[dcharkes]

Okay, you have convinced me. Less magic is better.

We can. The parameter just need a default value, which you can't see in an abstract or external function, because that's an implementation detail.

We cannot supply a default value in the patch file, because it would need to be Uint8List(sizeOf<subtype>()). So we would still need to transform the calls to super() in subtypes of Struct. But that make the constructors themselves less magic, only the super call would contain magic.

Let me give this approach a spin.

@mkustermann Are you concerned about typed data views not being optimized away as said above?

Edit: structs don't save their offset internally either. So calling it with offset != 0 will create a view. We should ensure such views get optimized away.

[dcharkes]

Discussion from https://dart-review.googlesource.com/c/sdk/+/342763/13/tests/ffi/structs_typed_data_test.dart, we should consider using toplevel methods rather than super constructors to indicate that objects are also instantiated in other ways than the user defined constructor.

class Point extends Struct {
  factory Point() {
    final result = create<Point>();
    // ...
  }

  factory Point.fromTypedData(TypedData typedData) {
    final result = create<Point>(typedData);
    // ...
  }
}

This would absolutely make it clear that no user-constructors are run on Pointer<Point> p; p.ref or struct-by-value returns of Point. With the user-defined constructors (especially if they only have optional arguments), it could be ambiguous whether the user-defined constructor will be run. To be clear, the user-defined constructors are never run unless invoked directly by user code.

[mkustermann]

Moving the discussion from cl/342763 to here.

While thinking about this more, I have some reasons to think using constructors may not be ideal:

• If we allow new Point() with (user-defined constructors) to allocate & initialize a Point, I would expect that new Array<Point>() to allocate & initialize an array of points (i.e. call the user-defined Point constructor for individual fields), which it wouldn't with current proposal I guess
• I'd assume that a new Foo() will ensure that the Foo object is transitively initialized. That means a user-defined Foo constructor has to explicitly call the this..field = FieldConstructor() to get fields initialized (C++ would call default constructors automatically), it's explicit verbose user code - which if not done - would give very weird behavior from callers perspective
• Even though a struct may only have one user-defined constructor, objects will be created without using that constructor (e.g. when via FFI calls or memory) which is very counter-intuitive semantics
• It makes it rather verbose if any struct needs these extra constructors.
• We can no longer separate allocation from initialization: In C one can allocate memory in one place, pass it around to other code that will initialize it. With this scheme we'll force one round of initialization in the constructor, even if values get overridden shortly by other initialization code.
• The functionality we want to enable is to provide a way to create these native types but don't care about memory management as they are only used by-value in FFI (in FFI calls or memory stores). Right now users would: allocate<Foo>().ref and later on free the pointer. Now we'd use GC-managed data for the struct so it doesn't have to be freed. So it would seem having create<Foo>() is the dual to allocate<Foo>().ref with automatic memory management. It also allows that any initialization code can be used for Foos created in Dart and Foos created via allocate.

For these reasons I think the concept of generative constructors on native types is a little iffy and it may make more sense to provide a create<>() / createArray<>() functionality (dual to allocate() / allocate(int numElements))

[dcharkes]

We have decided to go with Struct.create and Union.create.

Downsides that we find acceptable:

• More custom checking in the CFE/analyzer. Struct.create<Struct> and Struct.create<T> where T is a generic need to be rejected. (With super constructor calls this was not an issue, the callee was always exactly a know subtype of struct.)
• Keeping the unnamed constructor in Struct, and giving an error message on trying to invoke it or implicit unnamed constructors in subclasses in the CFE/analzyer.
• Conciseness of user-defined (factory) constructors. The super invocation would be implicit in a generative constructor, while a create call is always explicit:

final class Coordinate extends Struct {
  Coordinate({double? x, double? y}) {
    if (x != null) this.x = x;
    if (y != null) this.y = y;
  }

  Coordinate.fromTypedList(super.typedList);
  
  // ...
}

// vs

final class Coordinate extends Struct {
  factory Coordinate({double? x, double? y}) {
    final result = Struct.create<Coordinate>();
    if (x != null) result.x = x;
    if (y != null) result.y = y;
    return result;
  }

  factory Coordinate.fromTypedList(TypedData typedList) {
    return Struct.create<Coordinate>(typedList);
  }

  // ...
}

Full diff for the interested: https://dart-review.googlesource.com/c/sdk/+/342763/14..16

The upsides of create as outlined above outweigh the downsides.

(Also, if we'd wanted to explore making the structs actual extension types, in a potential dart:ffi2, then not having constructors is a step in the right direction. #54504)