Skip to content

[vm/ffi] Performance cliff for nested structs and arrays if used both with Pointer and TypedData backing store #54892

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

Closed
dcharkes opened this issue Feb 12, 2024 · 1 comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi triaged Issue has been triaged by sub team type-performance Issue relates to performance or code size

Comments

@dcharkes
Copy link
Contributor

/// Generates an expression that returns a new `TypedDataBase` offset
/// by [offset] from [typedDataBase].
///
/// If [typedDataBase] is a `Pointer`, returns a `Pointer<dartType>`.
/// If [typedDataBase] is a `TypedData` returns a `TypedData`.
///
/// Sample output:
///
/// ```
/// Object #typedDataBase = typedDataBase;
/// int #offset = offset;
/// #typedDataBase is Pointer ?
/// _pointerOffset<dartType>(#typedDataBase, #offset) :
/// _typedDataOffset((#typedDataBase as TypedData), #offset, length)
/// ```
Expression typedDataBaseOffset(Expression typedDataBase, Expression offset,

This pattern of checking for whether something is backed by a Pointer, and doing pointer arithmetic if it is a pointer and a typed data view if it is not a pointer causes a performance cliff when TFA cannot prove that one of the branches is definitely not happening.

ddfc00e caused benchmarks/FfiStructCopy/dart/FfiStructCopy.dart to slow down 15x. This is because the TypedData path in the if is no longer removed by TFA (due to the TypedData constructors for Array now being shared). This causes the inliner to bail out on inlining all Pointer and TypedData view calculations. Which in turn causes a bunch of calls and allocations.

I presume that any reasonably large FFI program would have both Pointer and TypedDatas backing structs, so this micro-benchmark was probably giving us a too rosy view. (Because if this, I don't believe a revert is in order.)

We should try to remove any branching on something being a Pointer or TypedData.

Some ideas:

  • a. Making a recognized method that does a typedDataBase.offsetBy and calling that in the struct/array implementation instead of branching on an instanceOf<Pointer>. The question is what IL/machine code to generate in this case. We'd still need to branch on whether to allocate a Pointer object or an TypedDataView object.
  • b. Make Structs/Arrays always work on external typed data and never use Pointers.
  • c. Have an offset field in _Compound and never create new typed data views or pointers at all. Nested struct and array accessors would return the same typed data base object but with a different int offset.

cc @mkustermann I believe you have argued for option (c) before.

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-performance Issue relates to performance or code size library-ffi labels Feb 12, 2024
@a-siva a-siva added the triaged Issue has been triaged by sub team label Feb 14, 2024
@dcharkes
Copy link
Contributor Author

Performance regression has been addressed: https://golem.corp.goog/Revision?repository=dart&revision=109399

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 triaged Issue has been triaged by sub team type-performance Issue relates to performance or code size
Projects
None yet
Development

No branches or pull requests

2 participants