[vm/ffi] Remove memory management from API

[sjindel-google]

Presently, the Pointer class contains wrappers for malloc and free.

However, we should consider a more neutral position on memory management. It doesn't makes sense to expose these as primitives in WebAssembly, and we can already implement them in Dart.

We can:

1. Remove Pointer.allocate and Pointer.free and move them to package:ffi.
2. Introduce an Allocator API, e.g.:

class Allocator {
  Pointer<Void> allocate(int bytes);
  void deallocate(Pointer<Void> memory);
}

However, the Allocator API should arguably live in package:ffi anyway. That would facilitate using dependency injection to mock or replace the allocator and implement different allocation strategies.

[sjindel-google]

/cc @mkustermann @dcharkes

If we change this, we would probably do it before the next release or we risk breaking all our users.

[dcharkes]

Yeah, good idea. Would you like to make a design for this that we could discuss?

If we change this, we would probably do it before the next release or we risk breaking all our users.

Definitely. Even though we're really explicit on that this is an experimental feature and that the API might/will change, I agree that it's important to prioritize things that change the API.

/cc @mraleph @mit-mit

[mkustermann]

Instead of removing it immediately, we can also mark it as @deprecated for some time, which will show up in the editor and would give people time to transition.

[sjindel-google]

My design is just that we remove Pointer.allocate and Pointer.free from our API.

[dcharkes]

My design is just that we remove Pointer.allocate and Pointer.free from our API.

Does this have impact on user code besides changing allocate and free to be looked up through lookupFunction?

[sjindel-google]

Not as far as I know.

[dcharkes]

This would make the following 3 issues obsolete:

• [vm/ffi] Write test for behavior of allocation of size-0 structs. #37265 [vm/ffi] Write test for behavior of allocation of size-0 structs.
• FFI: allocate can return negative pointers on 32-bit systems #36311 FFI: allocate can return negative pointers on 32-bit systems
• [vm/ffi] Test FFI with high heap #37251 [vm/ffi] Test FFI with high heap

[mkustermann]

We should ensure we can find the right malloc and free on all platforms. Then I don't see any reason why we shouldn't move ahead with this.

[sjindel-google]

It turns out that using malloc and free is already broken on Windows, because we statically link CRT (at least the standalone embedder does):

https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-library-features?view=vs-2019#what-problems-exist-if-an-application-uses-more-than-one-crt-version

We must either remove Pointer.allocate/Pointer.free or re-wire them to HeapAlloc/HeapFree.

[sjindel-google]

allocate and free have been deprecated and their replacements are available in package:ffi.

We just need https://github.com/dart-lang/tflite_native/pull/29/files to land before we can remove them entirely.

[mkustermann]

The PR for package:tflite_native has landed. Can we then remove those deprecated functions?

[sjindel-google]

This is blocked on non-generic structs in the SDK, since we need to roll the latest package:ffi and package:tflite_native to fully remove the deprecated methods.

[dcharkes]

We can remove all deprecated methods together, tracked in issue #38860.

I'll create a CL that removes all of them. (Unless you already have a CL to remove the memory management ones @sjindel-google?)