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

Improve performance of FFI calls that use many handles #47624

Closed
mkustermann opened this issue Nov 4, 2021 · 1 comment
Closed

Improve performance of FFI calls that use many handles #47624

mkustermann opened this issue Nov 4, 2021 · 1 comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@mkustermann
Copy link
Member

When passing many handles to C functions the handle allocation becomes a bottleneck. We'd therefore like to improve the performance of the simple implementation done in 6544c69.

@mkustermann mkustermann added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels Nov 4, 2021
@mkustermann mkustermann assigned ghost May 4, 2022
copybara-service bot pushed a commit that referenced this issue May 11, 2022
Passing handles in FFI calls has significant overhead due to
how each handle requires a runtime entry to allocate in the
handle scope.

This change removes that runtime entry by relying on the
register allocator to allocate all handle arguments on the stack,
so that we don't need to allocate them separately.
To pass the stack handles to the native call we then pass a pointer
to the stack slot as the native argument.

Testing:
- We already have comprehensive tests for correctness in the form
  of the FFI tests. These make calls with various combinations of
  Handle and non-handle arguments.
- Correct GC behaviour is likewise covered in
  `vmspecific_handle_test.dart` which makes calls with handles
  arguments and triggers GC in-flight.
  In case we do not correctly pass the handles on the stack, the
  GC will trash them during the call.

TEST=Existing.
Bug: #47624
Change-Id: Ic837bad5484daaa5534b7c2e8707ac2c5dfa480f
Cq-Include-Trybots: luci.dart.try:vm-kernel-gcc-linux-try,vm-kernel-linux-debug-simriscv64-try,vm-kernel-nnbd-mac-release-arm64-try,vm-kernel-precomp-android-release-arm64c-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-simriscv64-try,vm-precomp-ffi-qemu-linux-release-arm-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243320
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
@ghost ghost removed their assignment May 16, 2022
@mkustermann
Copy link
Member Author

Closing in favor of #42341

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

1 participant