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

Remove all unnecessary Enter/ExitHandleScope()s in FfiCalls #48989

Open
ghost opened this issue May 10, 2022 · 1 comment
Open

Remove all unnecessary Enter/ExitHandleScope()s in FfiCalls #48989

ghost opened this issue May 10, 2022 · 1 comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. library-ffi type-performance Issue relates to performance or code size

Comments

@ghost
Copy link

ghost commented May 10, 2022

All FfiCalls internally call Enter- and ExitHandleScope() for all calls that contain handles in the function signature.
This behaviour will no longer be needed for Handle arguments as of 243320, but removing this behaviour is a breaking change since the native function might rely on there being a scope, to e.g. call Dart API, like Dart_NewSendPort.

We need to investigate how we can best get rid of all unnecessary scopes.
This could for instance be:

  • Making a breaking change, and have all users migrate their code to manually set up the necessary scopes. Or ..
  • Have an opt-in, like @FfiNative<>(.., requiresScope:false).
@ghost ghost added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. type-performance Issue relates to performance or code size library-ffi labels May 10, 2022
@dcharkes
Copy link
Contributor

Adding a parameter sounds like a good idea.

We would have to experiment a bit with this a breaking change, to see how it breaks for user code if they don't set up the scope.

  • Silently allocating everything in the top scope having a million handles there would be bad.
  • A crash would be good.

Also, we might want to warn on returning a handle and having requiresScope:false because that handle would not be freed from the top handle scope. (Or alternatively, we want to free that handle on return, but that would also be a call to a dynamically linked runtime entry.)

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, FFI, and the AOT and JIT backends. library-ffi type-performance Issue relates to performance or code size
Projects
None yet
Development

No branches or pull requests

1 participant