Skip to content
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

[vm/ffi] Add Pointer<Struct>.value = for copying full structs #44768

Closed
dcharkes opened this issue Jan 25, 2021 · 3 comments
Closed

[vm/ffi] Add Pointer<Struct>.value = for copying full structs #44768

dcharkes opened this issue Jan 25, 2021 · 3 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Jan 25, 2021

We do not have a way of bulk-copying the full contents of a Struct in dart:ffi.

One use case where this is creating the need for boilerplate is when getting passed a struct by value from C, and having to pass back a pointer to a struct. @TimWhiting can you quantify how common this pattern is? #36730 (comment)

A workaround currently is to define a struct that nests the struct by value.

class MyStruct {
  // Many native fields.
}

class MyStruct2 {
  MyStruct myStruct
}

Pointer<MyStruct> allocateAndCopy(MyStruct source, Allocator allocator){
  final result = allocator<MyStruct>();
  result.cast<MyStruct2>().ref.myStruct = source;
  return result;
}

This accesses the TypedData and/or Pointer objects wrapped by the Structs and uses sizeOf<NativeType>() and _memCopy to copy the bytes:

void _memCopy(Object target, int targetOffsetInBytes, Object source,
int sourceOffsetInBytes, int lengthInBytes) {
assert(source is Pointer || source is TypedData);
assert(target is Pointer || target is TypedData);
if (source is Pointer) {
final sourcePointer = source.cast<Uint8>();
if (target is Pointer) {
final targetPointer = target.cast<Uint8>();
for (int i = 0; i < lengthInBytes; i++) {
targetPointer[i + targetOffsetInBytes] =
sourcePointer[i + sourceOffsetInBytes];
}
} else if (target is TypedData) {
final targetTypedData = target.buffer.asUint8List(target.offsetInBytes);
for (int i = 0; i < lengthInBytes; i++) {
targetTypedData[i + targetOffsetInBytes] =
sourcePointer[i + sourceOffsetInBytes];
}
}
} else if (source is TypedData) {
final sourceTypedData = source.buffer.asUint8List(source.offsetInBytes);
if (target is Pointer) {
final targetPointer = target.cast<Uint8>();
for (int i = 0; i < lengthInBytes; i++) {
targetPointer[i + targetOffsetInBytes] =
sourceTypedData[i + sourceOffsetInBytes];
}
} else if (target is TypedData) {
final targetTypedData = target.buffer.asUint8List(target.offsetInBytes);
targetTypedData.setRange(
targetOffsetInBytes,
targetOffsetInBytes + lengthInBytes,
sourceTypedData.sublist(sourceOffsetInBytes));
}
}
}

We should enable using this _memCopy for full Structs instead of only fields.

sdk/sdk/lib/ffi/ffi.dart

Lines 540 to 551 in ba01596

/// Extension on [Pointer] specialized for the type argument [Struct].
extension StructPointer<T extends Struct> on Pointer<T> {
/// Creates a reference to access the fields of this struct backed by native
/// memory at [address].
///
/// The [address] must be aligned according to the struct alignment rules of
/// the platform.
///
/// Support for invoking this extension method with non-constant [T] will be
/// removed in the next stable version of Dart and it will become mandatory
/// to invoke it with a compile-time constant [T].
external T get ref;

extension StructPointer<T extends Struct> on Pointer<T> {
  /// This extension method must be invoked with a compile-time constant [T].
  external void set value(T value); 
}

For the specific use case the code would look like:

final pointer = allocator<MyStruct>()..value = source;
@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels Jan 25, 2021
@dcharkes dcharkes changed the title [vm/ffi] Add Pointer<Struct>.ref = for copying full structs [vm/ffi] Add Pointer<Struct>.value = for copying full structs Jan 25, 2021
@dcharkes
Copy link
Contributor Author

@lrhn what do you think about .value = ...? Using .ref = ... is not really appropriate, but it would be symmetric with the getter.

@TimWhiting
Copy link

TimWhiting commented Jan 25, 2021

@dcharkes
Sorry for the delayed response. I see it at least 30 times in this library. So it is fairly common, but not as common as I thought. I think this still would be a nice addition to the ffi api making the Pointer api more complete.

For reference: I cloned the cpp client library: https://github.com/ros2/rclcpp
and then did the following two commands:
grep -rE '\*.+=[ ]*rcl_.*' .
grep -rE '\*.+=[ ]*rmw_.*' .

The c library has functions that start with rcl and rmw, and the struct values are assigned to pointer dereferences which is detected by the regex. There is one false positive in the grep I think, but I might be missing a few when the rcl or rmw gets wrapped onto the next line.

@richardheap
Copy link

@dcharkes I found a similar example to @TimWhiting 's use case. This one comes from the AWS C core library that, for me, is used by the AWS encryption SDK (partial, but sufficient and workable, implementation coming to pub.dev shortly).

From its hash table implementation: https://github.com/awslabs/aws-c-common/blob/main/include/aws/common/hash_table.h

The function to begin iteration of a map returns a structure by value, but the done and next functions expect that passed back by pointer.

I could wrap this in my own function, but the SDK already builds dynamic libraries for Windows, Linux and MacOS which I've been able to use directly (up to this point) by simply loading the library. I can make do without this iteration right now, but I remembered you'd wanted examples.

struct aws_hash_iter {
    const struct aws_hash_table *map;
    struct aws_hash_element element;
    size_t slot;
    size_t limit;
    enum aws_hash_iter_status status;
    // some lines deleted
};

/**
 * Returns an iterator to be used for iterating through a hash table.
 * Iterator will already point to the first element of the table it finds,
 * which can be accessed as iter.element.
 *
 * This function cannot fail, but if there are no elements in the table,
 * the returned iterator will return true for aws_hash_iter_done(&iter).
 */
AWS_COMMON_API
struct aws_hash_iter aws_hash_iter_begin(const struct aws_hash_table *map);

/**
 * Returns true if iterator is done iterating through table, false otherwise.
 * If this is true, the iterator will not include an element of the table.
 */
AWS_COMMON_API
bool aws_hash_iter_done(const struct aws_hash_iter *iter);

/**
 * Updates iterator so that it points to next element of hash table.
 *
 * This and the two previous functions are designed to be used together with
 * the following idiom:
 *
 * for (struct aws_hash_iter iter = aws_hash_iter_begin(&map);
 *      !aws_hash_iter_done(&iter); aws_hash_iter_next(&iter)) {
 *     const key_type key = *(const key_type *)iter.element.key;
 *     value_type value = *(value_type *)iter.element.value;
 *     // etc.
 * }
 *
 * Note that calling this on an iter which is "done" is idempotent:
 * i.e. it will return another iter which is "done".
 */
AWS_COMMON_API
void aws_hash_iter_next(struct aws_hash_iter *iter);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi
Projects
None yet
Development

No branches or pull requests

3 participants