From 92842351054846de4f141fa0cd77dba3076f1365 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 18 May 2022 11:33:26 +0200 Subject: [PATCH 1/6] Refactor methods of Managed_Resource Promoted `with`, `take`, `finalize` to be methods of Managed_Resource rather than static methods always taking `resource`, for consistency reasons. This required function dispatch boilerplate, similarly to `Ref`. In future iterations we will address this boilerplate code. Related to https://www.pivotaltracker.com/story/show/182212217 --- .../Base/0.0.0-dev/src/Runtime/Resource.enso | 19 ++----- .../Base/0.0.0-dev/src/System/File.enso | 14 ++--- .../0.0.0-dev/src/Connection/Connection.enso | 4 +- docs/semantics/managed-resources.md | 4 +- .../builtin/resource/FinalizeNode.java | 6 +- .../expression/builtin/resource/TakeNode.java | 8 +-- .../expression/builtin/resource/WithNode.java | 9 +-- .../runtime/data/ManagedResource.java | 57 +++++++++++++++++++ .../test/semantic/RuntimeManagementTest.scala | 10 ++-- 9 files changed, 87 insertions(+), 44 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Resource.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Resource.enso index b7db4c23f562..a96f5e2f5238 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Resource.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Resource.enso @@ -36,7 +36,6 @@ type Managed_Resource function once it is no longer in use. Arguments: - - resource: The resource to be managed automatically. - function: The action to be executed on resource to clean it up when it is no longer in use. register : Any -> (Any -> Nothing) -> Managed_Resource @@ -46,11 +45,8 @@ type Managed_Resource Forces finalization of a managed resource using the registered finalizer, even if the resource is still reachable. - - Arguments: - - resource: The resource that should be finalized. - finalize : Managed_Resource -> Nothing - finalize resource = @Builtin_Method "Managed_Resource.finalize" + finalize : Nothing + finalize = @Builtin_Method "Managed_Resource.finalize" ## ADVANCED @@ -58,20 +54,15 @@ type Managed_Resource resource object. Arguments: - - resource: The managed resource on which to run the action. - action: The action that will be applied to the resource managed by resource. - with : Managed_Resource -> (Any -> Any) -> Any - with resource ~action = @Builtin_Method "Managed_Resource.with" + with : (Any -> Any) -> Any + with ~action = @Builtin_Method "Managed_Resource.with" ## ADVANCED Takes the value held by the managed resource and unregisters the finalization step for this resource, effectively removing it from the managed resources system. - - Arguments: - - resource: The managed resource from which to acquire the underlying - resource. take : Managed_Resource -> Any - take resource = @Builtin_Method "Managed_Resource.take" + take = @Builtin_Method "Managed_Resource.take" diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso index f68fe8d7a40b..f92dd21f3f22 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso @@ -727,7 +727,7 @@ type Output_Stream out_stream.write_bytes "hello".utf_8 out_stream.close write_bytes : Vector.Vector -> Nothing ! File_Error - write_bytes contents = Managed_Resource.with this.stream_resource java_stream-> + write_bytes contents = this.stream_resource . with java_stream-> here.handle_java_exceptions this.file <| java_stream.write contents.to_array java_stream.flush @@ -752,7 +752,7 @@ type Output_Stream out_stream = file.new_output_stream [Option.Create] out_stream.close close : Nothing - close = Managed_Resource.finalize this.stream_resource + close = this.stream_resource . finalize ## An input stream, allowing for interactive reading of contents from an open file. @@ -786,7 +786,7 @@ type Input_Stream in_stream.close bytes read_all_bytes : Vector.Vector ! File_Error - read_all_bytes = Managed_Resource.with this.stream_resource java_stream-> + read_all_bytes = this.stream_resource . with java_stream-> here.handle_java_exceptions this.file <| Vector.Vector java_stream.readAllBytes @@ -816,7 +816,7 @@ type Input_Stream in_stream.close bytes read_n_bytes : Integer -> Vector.Vector ! File_Error - read_n_bytes n = Managed_Resource.with this.stream_resource java_stream-> + read_n_bytes n = this.stream_resource . with java_stream-> here.handle_java_exceptions this.file <| bytes = java_stream.readNBytes n Vector.Vector bytes @@ -841,7 +841,7 @@ type Input_Stream in_stream.close bytes read_byte : Integer ! File_Error - read_byte = Managed_Resource.with this.stream_resource java_stream-> + read_byte = this.stream_resource . with java_stream-> here.handle_java_exceptions this.file <| java_stream.read @@ -864,7 +864,7 @@ type Input_Stream in_stream = file.new_input_stream [Option.Read] in_stream.close close : Nothing - close = Managed_Resource.finalize this.stream_resource + close = this.stream_resource . finalize ## PRIVATE @@ -876,7 +876,7 @@ type Input_Stream Useful when integrating with polyglot functions requiring an `InputStream` as an argument. with_java_stream : (Java_Input_Stream -> Any) -> Any - with_java_stream f = Managed_Resource.with this.stream_resource f + with_java_stream f = this.stream_resource . with f ## PRIVATE diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso index 03b3883af66d..7e369855777a 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso @@ -54,7 +54,7 @@ type Connection The connection is not usable afterwards. close : Nothing close = - Managed_Resource.finalize this.connection_resource + this.connection_resource . finalize ## ADVANCED @@ -109,7 +109,7 @@ type Connection information to any thrown SQL errors. with_prepared_statement : Text | Sql.Statement -> (PreparedStatement -> Any) -> Any with_prepared_statement query action = - prepare template holes = Managed_Resource.with this.connection_resource java_connection-> + prepare template holes = this.connection_resource . with java_connection-> stmt = java_connection.prepareStatement template Panic.catch Any (here.set_statement_values stmt holes) caught_panic-> stmt.close diff --git a/docs/semantics/managed-resources.md b/docs/semantics/managed-resources.md index e9b7dae07044..4e55cac40fc9 100644 --- a/docs/semantics/managed-resources.md +++ b/docs/semantics/managed-resources.md @@ -51,7 +51,7 @@ call. > `Managed_Resource.register` call. To perform operations on the underlying resource, use the -`Managed_Resource.with resource action` method, where `resource` is the object +`.with action` method, where `` is the object returned from the call to `Managed_Resource.register`, and `action` is a function taking the underlying object as its only argument. It is important that the object passed to `action` is not stored and is not used past the return of @@ -61,7 +61,7 @@ reference to that object, if the thread remains alive past the return of itself, using a reference to the original manged resource. A managed resource can be closed manually, using -`Managed_Resource.close resource`. The underlying object is then finalized +`.close`. The underlying object is then finalized immediately. The finalization of a resource can be aborted using diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/FinalizeNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/FinalizeNode.java index f5141de68a28..0c9c3d54b717 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/FinalizeNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/FinalizeNode.java @@ -16,12 +16,12 @@ static FinalizeNode build() { return FinalizeNodeGen.create(); } - abstract Object execute(Object _this, ManagedResource resource); + abstract Object execute(Object _this); @Specialization - Object doClose(Object _this, ManagedResource resource) { + Object doClose(ManagedResource _this) { Context context = Context.get(this); - context.getResourceManager().close(resource); + context.getResourceManager().close(_this); return context.getBuiltins().nothing().newInstance(); } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/TakeNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/TakeNode.java index 2c791295fea0..bacc19749f54 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/TakeNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/TakeNode.java @@ -18,11 +18,11 @@ static TakeNode build() { return TakeNodeGen.create(); } - abstract Object execute(Object _this, ManagedResource resource); + abstract Object execute(Object _this); @Specialization - Object doTake(Object _this, ManagedResource resource) { - Context.get(this).getResourceManager().take(resource); - return resource.getResource(); + Object doTake(ManagedResource _this) { + Context.get(this).getResourceManager().take(_this); + return _this.getResource(); } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java index f26d3ebb1080..f8bf8dd76b2a 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java @@ -30,15 +30,10 @@ static WithNode build() { } abstract Stateful execute( - @MonadicState Object state, - VirtualFrame frame, - Object _this, - ManagedResource resource, - Object action); + @MonadicState Object state, VirtualFrame frame, Object _this, Object action); @Specialization - Stateful doWith( - Object state, VirtualFrame frame, Object _this, ManagedResource resource, Object action) { + Stateful doWith(Object state, VirtualFrame frame, ManagedResource resource, Object action) { ResourceManager resourceManager = Context.get(this).getResourceManager(); resourceManager.park(resource); try { diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ManagedResource.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ManagedResource.java index fde2b7a953e3..81fbf368245e 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ManagedResource.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/ManagedResource.java @@ -1,10 +1,20 @@ package org.enso.interpreter.runtime.data; +import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.dsl.Cached; +import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.interop.TruffleObject; +import com.oracle.truffle.api.library.ExportLibrary; +import com.oracle.truffle.api.library.ExportMessage; +import org.enso.interpreter.runtime.Context; +import org.enso.interpreter.runtime.callable.UnresolvedSymbol; +import org.enso.interpreter.runtime.callable.function.Function; +import org.enso.interpreter.runtime.library.dispatch.MethodDispatchLibrary; import java.lang.ref.PhantomReference; /** A runtime representation of a managed resource. */ +@ExportLibrary(MethodDispatchLibrary.class) public class ManagedResource implements TruffleObject { private final Object resource; private PhantomReference phantomReference; @@ -37,4 +47,51 @@ public PhantomReference getPhantomReference() { public void setPhantomReference(PhantomReference phantomReference) { this.phantomReference = phantomReference; } + + @ExportMessage + boolean hasFunctionalDispatch() { + return true; + } + + @ExportMessage + static class GetFunctionalDispatch { + + static final int CACHE_SIZE = 10; + + @CompilerDirectives.TruffleBoundary + static Function doResolve(UnresolvedSymbol symbol) { + Context context = getContext(); + return symbol.resolveFor( + context.getBuiltins().managedResource(), context.getBuiltins().any()); + } + + static Context getContext() { + return Context.get(null); + } + + @Specialization( + guards = { + "!getContext().isInlineCachingDisabled()", + "cachedSymbol == symbol", + "function != null" + }, + limit = "CACHE_SIZE") + static Function resolveCached( + ManagedResource _this, + UnresolvedSymbol symbol, + @Cached("symbol") UnresolvedSymbol cachedSymbol, + @Cached("doResolve(cachedSymbol)") Function function) { + return function; + } + + @Specialization(replaces = "resolveCached") + static Function resolve(ManagedResource _this, UnresolvedSymbol symbol) + throws MethodDispatchLibrary.NoSuchMethodException { + Function function = doResolve(symbol); + if (function == null) { + throw new MethodDispatchLibrary.NoSuchMethodException(); + } + return function; + } + } } diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala index 46cb270403fc..290ea01d9948 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala @@ -87,7 +87,7 @@ class RuntimeManagementTest extends InterpreterTest { |create_resource i = | c = Mock_File i | r = Managed_Resource.register c here.free_resource - | Managed_Resource.with r f-> IO.println ("Accessing: " + f.to_text) + | r . with f-> IO.println ("Accessing: " + f.to_text) | |main = | here.create_resource 0 @@ -127,8 +127,8 @@ class RuntimeManagementTest extends InterpreterTest { |create_resource i = | c = Mock_File i | r = Managed_Resource.register c here.free_resource - | Managed_Resource.with r f-> IO.println ("Accessing: " + f.to_text) - | if i % 2 == 0 then Managed_Resource.finalize r else Nothing + | r . with f-> IO.println ("Accessing: " + f.to_text) + | if i % 2 == 0 then r.finalize else Nothing | |main = | here.create_resource 0 @@ -168,8 +168,8 @@ class RuntimeManagementTest extends InterpreterTest { |create_resource i = | c = Mock_File i | r = Managed_Resource.register c here.free_resource - | Managed_Resource.with r f-> IO.println ("Accessing: " + f.to_text) - | if i % 2 == 0 then Managed_Resource.take r else Nothing + | r . with f-> IO.println ("Accessing: " + f.to_text) + | if i % 2 == 0 then r.take else Nothing | |main = | here.create_resource 0 From b4aae7d8cc296088b25eee37ebb4f50a07d69955 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 18 May 2022 11:41:24 +0200 Subject: [PATCH 2/6] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b91b114dd12..81e3d24d646d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -120,6 +120,8 @@ - [Implemented `compute` method on `Vector` for statistics calculations.][3442] - [Promote get and put to be methods of Ref type rather than of Ref module][3457] +- [Promote with, take, finalize to be methods of Managed_Resource + instance][3460] [debug-shortcuts]: https://github.com/enso-org/enso/blob/develop/app/gui/docs/product/shortcuts.md#debug @@ -186,6 +188,7 @@ [3430]: https://github.com/enso-org/enso/pull/3430 [3442]: https://github.com/enso-org/enso/pull/3442 [3457]: https://github.com/enso-org/enso/pull/3457 +[3460]: https://github.com/enso-org/enso/pull/3460 #### Enso Compiler From 84d4fa365f87d5c4dba0b5e71422efea642bcc43 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 18 May 2022 11:46:43 +0200 Subject: [PATCH 3/6] Minor inconsistency in parameter names --- .../node/expression/builtin/resource/WithNode.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java index f8bf8dd76b2a..639e49695a98 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java @@ -33,14 +33,14 @@ abstract Stateful execute( @MonadicState Object state, VirtualFrame frame, Object _this, Object action); @Specialization - Stateful doWith(Object state, VirtualFrame frame, ManagedResource resource, Object action) { + Stateful doWith(Object state, VirtualFrame frame, ManagedResource _this, Object action) { ResourceManager resourceManager = Context.get(this).getResourceManager(); - resourceManager.park(resource); + resourceManager.park(_this); try { return invokeCallableNode.execute( - action, frame, state, new Object[] {resource.getResource()}); + action, frame, state, new Object[] {_this.getResource()}); } finally { - resourceManager.unpark(resource); + resourceManager.unpark(_this); } } } From 00c6a7ae50171e419490e5edbe4f042d25232a3b Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 18 May 2022 13:17:14 +0200 Subject: [PATCH 4/6] Preetify --- docs/semantics/managed-resources.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/semantics/managed-resources.md b/docs/semantics/managed-resources.md index 4e55cac40fc9..bcd4237bd7ba 100644 --- a/docs/semantics/managed-resources.md +++ b/docs/semantics/managed-resources.md @@ -51,8 +51,8 @@ call. > `Managed_Resource.register` call. To perform operations on the underlying resource, use the -`.with action` method, where `` is the object -returned from the call to `Managed_Resource.register`, and `action` is a +`.with action` method, where `` is the +object returned from the call to `Managed_Resource.register`, and `action` is a function taking the underlying object as its only argument. It is important that the object passed to `action` is not stored and is not used past the return of `action`. This means in particular that it is unsafe to give another thread a @@ -60,9 +60,8 @@ reference to that object, if the thread remains alive past the return of `action`. If such an operation is necessary, the other thread should call `with` itself, using a reference to the original manged resource. -A managed resource can be closed manually, using -`.close`. The underlying object is then finalized -immediately. +A managed resource can be closed manually, using `.close`. The +underlying object is then finalized immediately. The finalization of a resource can be aborted using `Managed_Resource.take resource`. This call will abort any automatic From f0c2bbdd39cfab5b5a047cd9e52dde647727cbe1 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 18 May 2022 13:56:34 +0200 Subject: [PATCH 5/6] Missed one with usage --- .../Standard/Database/0.0.0-dev/src/Connection/Connection.enso | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso index 7e369855777a..67b64e1e397f 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso @@ -182,7 +182,7 @@ type Connection db_types = pairs.map p-> p.second.sql_type insert_query = this.dialect.generate_sql <| IR.Insert name pairs insert_template = insert_query.prepare.first - Managed_Resource.with this.connection_resource java_connection-> + this.connection_resource . with java_connection-> default_autocommit = java_connection.getAutoCommit java_connection.setAutoCommit False Resource.bracket Nothing (_ -> java_connection.setAutoCommit default_autocommit) _-> From 0c38360acba52fa2215323ecf88daa885d133271 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 18 May 2022 14:44:00 +0200 Subject: [PATCH 6/6] Make formatter happy --- .../interpreter/node/expression/builtin/resource/WithNode.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java index 639e49695a98..3e9f0e4e8968 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/resource/WithNode.java @@ -37,8 +37,7 @@ Stateful doWith(Object state, VirtualFrame frame, ManagedResource _this, Object ResourceManager resourceManager = Context.get(this).getResourceManager(); resourceManager.park(_this); try { - return invokeCallableNode.execute( - action, frame, state, new Object[] {_this.getResource()}); + return invokeCallableNode.execute(action, frame, state, new Object[] {_this.getResource()}); } finally { resourceManager.unpark(_this); }