From 73bd44d3638ba7aac553576f8fa19253ae3f75e0 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Fri, 15 Jan 2021 18:04:12 +0000 Subject: [PATCH] [vm/ffi] Improve `Pool` samples Addressing comments from https://dart-review.googlesource.com/c/sdk/+/177706/20 Change-Id: I0cf023a5613978eebcb4aca84c9db24796687602 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/179180 Commit-Queue: Daco Harkes Reviewed-by: Lasse R.H. Nielsen --- samples/ffi/resource_management/pool.dart | 140 +++++++++++++----- .../ffi/resource_management/pool_sample.dart | 21 ++- .../pool_zoned_sample.dart | 63 +++++--- samples_2/ffi/resource_management/pool.dart | 140 +++++++++++++----- .../ffi/resource_management/pool_sample.dart | 20 ++- .../pool_zoned_sample.dart | 63 +++++--- 6 files changed, 321 insertions(+), 126 deletions(-) diff --git a/samples/ffi/resource_management/pool.dart b/samples/ffi/resource_management/pool.dart index 3bc0e2a0b25..14e07f4d623 100644 --- a/samples/ffi/resource_management/pool.dart +++ b/samples/ffi/resource_management/pool.dart @@ -11,30 +11,47 @@ import 'package:ffi/ffi.dart'; import '../calloc.dart'; -/// Keeps track of all allocated memory and frees all allocated memory on -/// [releaseAll]. +/// An [Allocator] which frees all allocations at the same time. /// -/// Wraps an [Allocator] to do the actual allocation and freeing. +/// The pool allows you to allocate heap memory, but ignores calls to [free]. +/// Instead you call [releaseAll] to release all the allocations at the same +/// time. +/// +/// Also allows other resources to be associated with the pool, through the +/// [using] method, to have a release function called for them when the pool is +/// released. +/// +/// An [Allocator] can be provided to do the actual allocation and freeing. +/// Defaults to using [calloc]. class Pool implements Allocator { /// The [Allocator] used for allocation and freeing. final Allocator _wrappedAllocator; - Pool(this._wrappedAllocator); - /// Native memory under management by this [Pool]. final List> _managedMemoryPointers = []; /// Callbacks for releasing native resources under management by this [Pool]. final List _managedResourceReleaseCallbacks = []; - /// Allocates memory on the native heap by using the allocator supplied to - /// the constructor. + bool _inUse = true; + + /// Creates a pool of allocations. + /// + /// The [allocator] is used to do the actual allocation and freeing of + /// memory. It defaults to using [calloc]. + Pool([Allocator allocator = calloc]) : _wrappedAllocator = allocator; + + /// Allocates memory and includes it in the pool. + /// + /// Uses the allocator provided to the [Pool] constructor to do the + /// allocation. /// /// Throws an [ArgumentError] if the number of bytes or alignment cannot be /// satisfied. @override - Pointer allocate(int numBytes, {int? alignment}) { - final p = _wrappedAllocator.allocate(numBytes, alignment: alignment); + Pointer allocate(int byteCount, {int? alignment}) { + _ensureInUse(); + final p = _wrappedAllocator.allocate(byteCount, alignment: alignment); _managedMemoryPointers.add(p); return p; } @@ -43,6 +60,8 @@ class Pool implements Allocator { /// /// Executes [releaseCallback] on [releaseAll]. T using(T resource, Function(T) releaseCallback) { + _ensureInUse(); + releaseCallback = Zone.current.bindUnaryCallback(releaseCallback); _managedResourceReleaseCallbacks.add(() => releaseCallback(resource)); return resource; } @@ -53,59 +72,102 @@ class Pool implements Allocator { } /// Releases all resources that this [Pool] manages. - void releaseAll() { - for (final c in _managedResourceReleaseCallbacks) { - c(); + /// + /// If [reuse] is `true`, the pool can be used again after resources + /// have been released. If not, the default, then the [allocate] + /// and [using] methods must not be called after a call to `releaseAll`. + void releaseAll({bool reuse = false}) { + if (!reuse) { + _inUse = false; + } + while (_managedResourceReleaseCallbacks.isNotEmpty) { + _managedResourceReleaseCallbacks.removeLast()(); } - _managedResourceReleaseCallbacks.clear(); for (final p in _managedMemoryPointers) { _wrappedAllocator.free(p); } _managedMemoryPointers.clear(); } + /// Does nothing, invoke [releaseAll] instead. @override - void free(Pointer pointer) => throw UnsupportedError( - "Individually freeing Pool allocated memory is not allowed"); + void free(Pointer pointer) {} + + void _ensureInUse() { + if (!_inUse) { + throw StateError( + "Pool no longer in use, `releaseAll(reuse: false)` was called."); + } + } } -/// Creates a [Pool] to manage native resources. +/// Runs [computation] with a new [Pool], and releases all allocations at the end. /// -/// If the isolate is shut down, through `Isolate.kill()`, resources are _not_ cleaned up. -R using(R Function(Pool) f, [Allocator wrappedAllocator = calloc]) { - final p = Pool(wrappedAllocator); +/// If [R] is a [Future], all allocations are released when the future completes. +/// +/// If the isolate is shut down, through `Isolate.kill()`, resources are _not_ +/// cleaned up. +R using(R Function(Pool) computation, + [Allocator wrappedAllocator = calloc]) { + final pool = Pool(wrappedAllocator); + bool isAsync = false; try { - return f(p); + final result = computation(pool); + if (result is Future) { + isAsync = true; + return (result.whenComplete(pool.releaseAll) as R); + } + return result; } finally { - p.releaseAll(); + if (!isAsync) { + pool.releaseAll(); + } } } /// Creates a zoned [Pool] to manage native resources. /// -/// Pool is availabe through [currentPool]. -/// -/// Please note that all throws are caught and packaged in [RethrownError]. +/// The pool is availabe through [zonePool]. /// /// If the isolate is shut down, through `Isolate.kill()`, resources are _not_ cleaned up. -R usePool(R Function() f, [Allocator wrappedAllocator = calloc]) { - final p = Pool(wrappedAllocator); +R withZonePool(R Function() computation, + [Allocator wrappedAllocator = calloc]) { + final pool = Pool(wrappedAllocator); + var poolHolder = [pool]; + bool isAsync = false; try { - return runZoned(() => f(), - zoneValues: {#_pool: p}, - onError: (error, st) => throw RethrownError(error, st)); + return runZoned(() { + final result = computation(); + if (result is Future) { + isAsync = true; + result.whenComplete(pool.releaseAll); + } + return result; + }, zoneValues: {#_pool: poolHolder}); } finally { - p.releaseAll(); + if (!isAsync) { + pool.releaseAll(); + poolHolder.remove(pool); + } } } -/// The [Pool] in the current zone. -Pool get currentPool => Zone.current[#_pool]; - -class RethrownError { - dynamic original; - StackTrace originalStackTrace; - RethrownError(this.original, this.originalStackTrace); - toString() => """RethrownError(${original}) -${originalStackTrace}"""; +/// A zone-specific [Pool]. +/// +/// Asynchronous computations can share a [Pool]. Use [withZonePool] to create +/// a new zone with a fresh [Pool], and that pool will then be released +/// automatically when the function passed to [withZonePool] completes. +/// All code inside that zone can use `zonePool` to access the pool. +/// +/// The current pool must not be accessed by code which is not running inside +/// a zone created by [withZonePool]. +Pool get zonePool { + final List? poolHolder = Zone.current[#_pool]; + if (poolHolder == null) { + throw StateError("Not inside a zone created by `usePool`"); + } + if (!poolHolder.isEmpty) { + return poolHolder.single; + } + throw StateError("Pool as already been cleared with releaseAll."); } diff --git a/samples/ffi/resource_management/pool_sample.dart b/samples/ffi/resource_management/pool_sample.dart index ef8b4159478..1f751655de4 100644 --- a/samples/ffi/resource_management/pool_sample.dart +++ b/samples/ffi/resource_management/pool_sample.dart @@ -4,6 +4,7 @@ // // Sample illustrating resource management with an explicit pool. +import 'dart:async'; import 'dart:ffi'; import 'package:expect/expect.dart'; @@ -12,7 +13,7 @@ import 'pool.dart'; import 'utf8_helpers.dart'; import '../dylib_utils.dart'; -main() { +main() async { final ffiTestDynamicLibrary = dlopenPlatformSpecific("ffi_test_dynamic_library"); @@ -108,6 +109,24 @@ main() { } on Exception catch (e) { print("Caught exception: $e"); } + + /// [using] waits with releasing its resources until after [Future]s + /// complete. + List freed = []; + freeInt(int i) { + freed.add(i); + } + + Future myFutureInt = using((Pool pool) { + return Future.microtask(() { + pool.using(1, freeInt); + return 1; + }); + }); + + Expect.isTrue(freed.isEmpty); + await myFutureInt; + Expect.equals(1, freed.single); } /// Represents some opaque resource being managed by a library. diff --git a/samples/ffi/resource_management/pool_zoned_sample.dart b/samples/ffi/resource_management/pool_zoned_sample.dart index 4c99fdc3cfb..b5b9eeda973 100644 --- a/samples/ffi/resource_management/pool_zoned_sample.dart +++ b/samples/ffi/resource_management/pool_zoned_sample.dart @@ -12,7 +12,7 @@ import 'pool.dart'; import 'utf8_helpers.dart'; import '../dylib_utils.dart'; -main() { +main() async { final ffiTestDynamicLibrary = dlopenPlatformSpecific("ffi_test_dynamic_library"); @@ -20,9 +20,9 @@ main() { Void Function(Pointer, Pointer, IntPtr), void Function(Pointer, Pointer, int)>("MemMove"); - // To ensure resources are freed, wrap them in a [using] call. - usePool(() { - final p = currentPool(2); + // To ensure resources are freed, wrap them in a [withZonePool] call. + withZonePool(() { + final p = zonePool(2); p[0] = 24; MemMove(p.elementAt(1).cast(), p.cast(), sizeOf()); print(p[1]); @@ -31,24 +31,23 @@ main() { // Resources are freed also when abnormal control flow occurs. try { - usePool(() { - final p = currentPool(2); + withZonePool(() { + final p = zonePool(2); p[0] = 25; MemMove(p.elementAt(1).cast(), p.cast(), 8); print(p[1]); Expect.equals(25, p[1]); throw Exception("Some random exception"); }); - } on RethrownError catch (e) { - // Note that exceptions are wrapped when using zones. - print("Caught exception: ${e.original}"); + } catch (e) { + print("Caught exception: ${e}"); } // In a pool multiple resources can be allocated, which will all be freed // at the end of the scope. - usePool(() { - final p = currentPool(2); - final p2 = currentPool(2); + withZonePool(() { + final p = zonePool(2); + final p2 = zonePool(2); p[0] = 1; p[1] = 2; MemMove(p2.cast(), p.cast(), 2 * sizeOf()); @@ -59,10 +58,10 @@ main() { // If the resource allocation happens in a different scope, it is in the // same zone, so it's lifetime is automatically managed by the pool. f1() { - return currentPool(2); + return zonePool(2); } - usePool(() { + withZonePool(() { final p = f1(); final p2 = f1(); p[0] = 1; @@ -73,8 +72,8 @@ main() { }); // Using Strings. - usePool(() { - final p = "Hello world!".toUtf8(currentPool); + withZonePool(() { + final p = "Hello world!".toUtf8(zonePool); print(p.contents()); }); @@ -91,24 +90,42 @@ main() { void Function(Pointer)>("ReleaseResource"); // Using an FFI call to release a resource. - usePool(() { - final r = currentPool.using(allocateResource(), releaseResource); + withZonePool(() { + final r = zonePool.using(allocateResource(), releaseResource); useResource(r); }); // Using an FFI call to release a resource with abnormal control flow. try { - usePool(() { - final r = currentPool.using(allocateResource(), releaseResource); + withZonePool(() { + final r = zonePool.using(allocateResource(), releaseResource); useResource(r); throw Exception("Some random exception"); }); // Resource has been freed. - } on RethrownError catch (e) { - // Note that exceptions are wrapped when using zones. - print("Caught exception: ${e.original}"); + } catch (e) { + print("Caught exception: ${e}"); } + + /// [using] waits with releasing its resources until after [Future]s + /// complete. + + List freed = []; + freeInt(int i) { + freed.add(i); + } + + Future myFutureInt = withZonePool(() { + return Future.microtask(() { + zonePool.using(1, freeInt); + return 1; + }); + }); + + Expect.isTrue(freed.isEmpty); + await myFutureInt; + Expect.equals(1, freed.single); } /// Represents some opaque resource being managed by a library. diff --git a/samples_2/ffi/resource_management/pool.dart b/samples_2/ffi/resource_management/pool.dart index 477a78eb3d9..d4610b01502 100644 --- a/samples_2/ffi/resource_management/pool.dart +++ b/samples_2/ffi/resource_management/pool.dart @@ -13,30 +13,47 @@ import 'package:ffi/ffi.dart'; import '../calloc.dart'; -/// Keeps track of all allocated memory and frees all allocated memory on -/// [releaseAll]. +/// An [Allocator] which frees all allocations at the same time. /// -/// Wraps an [Allocator] to do the actual allocation and freeing. +/// The pool allows you to allocate heap memory, but ignores calls to [free]. +/// Instead you call [releaseAll] to release all the allocations at the same +/// time. +/// +/// Also allows other resources to be associated with the pool, through the +/// [using] method, to have a release function called for them when the pool is +/// released. +/// +/// An [Allocator] can be provided to do the actual allocation and freeing. +/// Defaults to using [calloc]. class Pool implements Allocator { /// The [Allocator] used for allocation and freeing. final Allocator _wrappedAllocator; - Pool(this._wrappedAllocator); - /// Native memory under management by this [Pool]. final List> _managedMemoryPointers = []; /// Callbacks for releasing native resources under management by this [Pool]. final List _managedResourceReleaseCallbacks = []; - /// Allocates memory on the native heap by using the allocator supplied to - /// the constructor. + bool _inUse = true; + + /// Creates a pool of allocations. + /// + /// The [allocator] is used to do the actual allocation and freeing of + /// memory. It defaults to using [calloc]. + Pool([Allocator allocator = calloc]) : _wrappedAllocator = allocator; + + /// Allocates memory and includes it in the pool. + /// + /// Uses the allocator provided to the [Pool] constructor to do the + /// allocation. /// /// Throws an [ArgumentError] if the number of bytes or alignment cannot be /// satisfied. @override - Pointer allocate(int numBytes, {int alignment}) { - final p = _wrappedAllocator.allocate(numBytes, alignment: alignment); + Pointer allocate(int byteCount, {int alignment}) { + _ensureInUse(); + final p = _wrappedAllocator.allocate(byteCount, alignment: alignment); _managedMemoryPointers.add(p); return p; } @@ -45,6 +62,8 @@ class Pool implements Allocator { /// /// Executes [releaseCallback] on [releaseAll]. T using(T resource, Function(T) releaseCallback) { + _ensureInUse(); + releaseCallback = Zone.current.bindUnaryCallback(releaseCallback); _managedResourceReleaseCallbacks.add(() => releaseCallback(resource)); return resource; } @@ -55,59 +74,102 @@ class Pool implements Allocator { } /// Releases all resources that this [Pool] manages. - void releaseAll() { - for (final c in _managedResourceReleaseCallbacks) { - c(); + /// + /// If [reuse] is `true`, the pool can be used again after resources + /// have been released. If not, the default, then the [allocate] + /// and [using] methods must not be called after a call to `releaseAll`. + void releaseAll({bool reuse = false}) { + if (!reuse) { + _inUse = false; + } + while (_managedResourceReleaseCallbacks.isNotEmpty) { + _managedResourceReleaseCallbacks.removeLast()(); } - _managedResourceReleaseCallbacks.clear(); for (final p in _managedMemoryPointers) { _wrappedAllocator.free(p); } _managedMemoryPointers.clear(); } + /// Does nothing, invoke [releaseAll] instead. @override - void free(Pointer pointer) => throw UnsupportedError( - "Individually freeing Pool allocated memory is not allowed"); + void free(Pointer pointer) {} + + void _ensureInUse() { + if (!_inUse) { + throw StateError( + "Pool no longer in use, `releaseAll(reuse: false)` was called."); + } + } } -/// Creates a [Pool] to manage native resources. +/// Runs [computation] with a new [Pool], and releases all allocations at the end. /// -/// If the isolate is shut down, through `Isolate.kill()`, resources are _not_ cleaned up. -R using(R Function(Pool) f, [Allocator wrappedAllocator = calloc]) { - final p = Pool(wrappedAllocator); +/// If [R] is a [Future], all allocations are released when the future completes. +/// +/// If the isolate is shut down, through `Isolate.kill()`, resources are _not_ +/// cleaned up. +R using(R Function(Pool) computation, + [Allocator wrappedAllocator = calloc]) { + final pool = Pool(wrappedAllocator); + bool isAsync = false; try { - return f(p); + final result = computation(pool); + if (result is Future) { + isAsync = true; + return (result.whenComplete(pool.releaseAll) as R); + } + return result; } finally { - p.releaseAll(); + if (!isAsync) { + pool.releaseAll(); + } } } /// Creates a zoned [Pool] to manage native resources. /// -/// Pool is availabe through [currentPool]. -/// -/// Please note that all throws are caught and packaged in [RethrownError]. +/// The pool is availabe through [zonePool]. /// /// If the isolate is shut down, through `Isolate.kill()`, resources are _not_ cleaned up. -R usePool(R Function() f, [Allocator wrappedAllocator = calloc]) { - final p = Pool(wrappedAllocator); +R withZonePool(R Function() computation, + [Allocator wrappedAllocator = calloc]) { + final pool = Pool(wrappedAllocator); + var poolHolder = [pool]; + bool isAsync = false; try { - return runZoned(() => f(), - zoneValues: {#_pool: p}, - onError: (error, st) => throw RethrownError(error, st)); + return runZoned(() { + final result = computation(); + if (result is Future) { + isAsync = true; + result.whenComplete(pool.releaseAll); + } + return result; + }, zoneValues: {#_pool: poolHolder}); } finally { - p.releaseAll(); + if (!isAsync) { + pool.releaseAll(); + poolHolder.remove(pool); + } } } -/// The [Pool] in the current zone. -Pool get currentPool => Zone.current[#_pool]; - -class RethrownError { - dynamic original; - StackTrace originalStackTrace; - RethrownError(this.original, this.originalStackTrace); - toString() => """RethrownError(${original}) -${originalStackTrace}"""; +/// A zone-specific [Pool]. +/// +/// Asynchronous computations can share a [Pool]. Use [withZonePool] to create +/// a new zone with a fresh [Pool], and that pool will then be released +/// automatically when the function passed to [withZonePool] completes. +/// All code inside that zone can use `zonePool` to access the pool. +/// +/// The current pool must not be accessed by code which is not running inside +/// a zone created by [withZonePool]. +Pool get zonePool { + final List poolHolder = Zone.current[#_pool]; + if (poolHolder == null) { + throw StateError("Not inside a zone created by `usePool`"); + } + if (!poolHolder.isEmpty) { + return poolHolder.single; + } + throw StateError("Pool as already been cleared with releaseAll."); } diff --git a/samples_2/ffi/resource_management/pool_sample.dart b/samples_2/ffi/resource_management/pool_sample.dart index c794a402490..e3441c18884 100644 --- a/samples_2/ffi/resource_management/pool_sample.dart +++ b/samples_2/ffi/resource_management/pool_sample.dart @@ -14,7 +14,7 @@ import 'pool.dart'; import 'utf8_helpers.dart'; import '../dylib_utils.dart'; -main() { +main() async { final ffiTestDynamicLibrary = dlopenPlatformSpecific("ffi_test_dynamic_library"); @@ -110,6 +110,24 @@ main() { } on Exception catch (e) { print("Caught exception: $e"); } + + /// [using] waits with releasing its resources until after [Future]s + /// complete. + List freed = []; + freeInt(int i) { + freed.add(i); + } + + Future myFutureInt = using((Pool pool) { + return Future.microtask(() { + pool.using(1, freeInt); + return 1; + }); + }); + + Expect.isTrue(freed.isEmpty); + await myFutureInt; + Expect.equals(1, freed.single); } /// Represents some opaque resource being managed by a library. diff --git a/samples_2/ffi/resource_management/pool_zoned_sample.dart b/samples_2/ffi/resource_management/pool_zoned_sample.dart index a1dc9c1e621..b9142c2ff1f 100644 --- a/samples_2/ffi/resource_management/pool_zoned_sample.dart +++ b/samples_2/ffi/resource_management/pool_zoned_sample.dart @@ -14,7 +14,7 @@ import 'pool.dart'; import 'utf8_helpers.dart'; import '../dylib_utils.dart'; -main() { +main() async { final ffiTestDynamicLibrary = dlopenPlatformSpecific("ffi_test_dynamic_library"); @@ -22,9 +22,9 @@ main() { Void Function(Pointer, Pointer, IntPtr), void Function(Pointer, Pointer, int)>("MemMove"); - // To ensure resources are freed, wrap them in a [using] call. - usePool(() { - final p = currentPool(2); + // To ensure resources are freed, wrap them in a [withZonePool] call. + withZonePool(() { + final p = zonePool(2); p[0] = 24; MemMove(p.elementAt(1).cast(), p.cast(), sizeOf()); print(p[1]); @@ -33,24 +33,23 @@ main() { // Resources are freed also when abnormal control flow occurs. try { - usePool(() { - final p = currentPool(2); + withZonePool(() { + final p = zonePool(2); p[0] = 25; MemMove(p.elementAt(1).cast(), p.cast(), 8); print(p[1]); Expect.equals(25, p[1]); throw Exception("Some random exception"); }); - } on RethrownError catch (e) { - // Note that exceptions are wrapped when using zones. - print("Caught exception: ${e.original}"); + } catch (e) { + print("Caught exception: ${e}"); } // In a pool multiple resources can be allocated, which will all be freed // at the end of the scope. - usePool(() { - final p = currentPool(2); - final p2 = currentPool(2); + withZonePool(() { + final p = zonePool(2); + final p2 = zonePool(2); p[0] = 1; p[1] = 2; MemMove(p2.cast(), p.cast(), 2 * sizeOf()); @@ -61,10 +60,10 @@ main() { // If the resource allocation happens in a different scope, it is in the // same zone, so it's lifetime is automatically managed by the pool. f1() { - return currentPool(2); + return zonePool(2); } - usePool(() { + withZonePool(() { final p = f1(); final p2 = f1(); p[0] = 1; @@ -75,8 +74,8 @@ main() { }); // Using Strings. - usePool(() { - final p = "Hello world!".toUtf8(currentPool); + withZonePool(() { + final p = "Hello world!".toUtf8(zonePool); print(p.contents()); }); @@ -93,24 +92,42 @@ main() { void Function(Pointer)>("ReleaseResource"); // Using an FFI call to release a resource. - usePool(() { - final r = currentPool.using(allocateResource(), releaseResource); + withZonePool(() { + final r = zonePool.using(allocateResource(), releaseResource); useResource(r); }); // Using an FFI call to release a resource with abnormal control flow. try { - usePool(() { - final r = currentPool.using(allocateResource(), releaseResource); + withZonePool(() { + final r = zonePool.using(allocateResource(), releaseResource); useResource(r); throw Exception("Some random exception"); }); // Resource has been freed. - } on RethrownError catch (e) { - // Note that exceptions are wrapped when using zones. - print("Caught exception: ${e.original}"); + } catch (e) { + print("Caught exception: ${e}"); } + + /// [using] waits with releasing its resources until after [Future]s + /// complete. + + List freed = []; + freeInt(int i) { + freed.add(i); + } + + Future myFutureInt = withZonePool(() { + return Future.microtask(() { + zonePool.using(1, freeInt); + return 1; + }); + }); + + Expect.isTrue(freed.isEmpty); + await myFutureInt; + Expect.equals(1, freed.single); } /// Represents some opaque resource being managed by a library.