From e1436772a84bda8f243bda38a1fb22f1ec8d5e3c Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 8 Sep 2023 13:14:42 -0500 Subject: [PATCH 01/18] Quick hacked up learning attempt --- py/server/deephaven/_wrapper.py | 16 ++++++++++++++++ py/server/deephaven/execution_context.py | 1 + py/server/deephaven/liveness_scope.py | 1 + py/server/deephaven/table.py | 1 + py/server/deephaven/table_listener.py | 10 ++++++++-- 5 files changed, 27 insertions(+), 2 deletions(-) diff --git a/py/server/deephaven/_wrapper.py b/py/server/deephaven/_wrapper.py index 47ed18dd019..c3212114410 100644 --- a/py/server/deephaven/_wrapper.py +++ b/py/server/deephaven/_wrapper.py @@ -22,6 +22,11 @@ _has_all_wrappers_imported = False JLivePyObjectWrapper = jpy.get_type('io.deephaven.server.plugin.python.LivePyObjectWrapper') +JLivenessScope = jpy.get_type("io.deephaven.engine.liveness.LivenessScope") +JLivenessReferent = jpy.get_type('io.deephaven.engine.liveness.LivenessReferent') + + +_interpreter_liveness_scope = JLivenessScope() def _recursive_import(package_path: str) -> None: @@ -53,6 +58,17 @@ def __init_subclass__(cls, *args, **kwargs): if _is_direct_initialisable(cls): _di_wrapper_classes.add(cls) + def __init__(self, j_object: jpy.JType): + self._j_obj = j_object + if JLivenessReferent.jclass.isInstance(self._j_obj): + print('retaining ', self._j_obj) + _interpreter_liveness_scope.tryManage(self._j_obj) + + def __del__(self): + if JLivenessReferent.jclass.isInstance(self._j_obj): + print("releasing ", self._j_obj) + _interpreter_liveness_scope.tryUnmanage(self._j_obj) + @property @abstractmethod def j_object(self) -> jpy.JType: diff --git a/py/server/deephaven/execution_context.py b/py/server/deephaven/execution_context.py index 8e416b9e229..2a020b6491d 100644 --- a/py/server/deephaven/execution_context.py +++ b/py/server/deephaven/execution_context.py @@ -42,6 +42,7 @@ def update_graph(self) -> UpdateGraph: return UpdateGraph(j_update_graph=self.j_exec_ctx.getUpdateGraph()) def __init__(self, j_exec_ctx): + super().__init__(j_exec_ctx) self.j_exec_ctx = j_exec_ctx def __enter__(self): diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 37dab61189e..5e6dd4d98dc 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -24,6 +24,7 @@ class LivenessScope(JObjectWrapper): j_object_type = _JLivenessScope def __init__(self, j_scope: jpy.JType): + super().__init__(j_scope) self.j_scope = j_scope def __enter__(self): diff --git a/py/server/deephaven/table.py b/py/server/deephaven/table.py index 15181e87691..62a4d99a6f0 100644 --- a/py/server/deephaven/table.py +++ b/py/server/deephaven/table.py @@ -507,6 +507,7 @@ class Table(JObjectWrapper): j_object_type = _J_Table def __init__(self, j_table: jpy.JType): + super().__init__(j_table) self.j_table = jpy.cast(j_table, self.j_object_type) if self.j_table is None: raise DHError("j_table type is not io.deephaven.engine.table.Table") diff --git a/py/server/deephaven/table_listener.py b/py/server/deephaven/table_listener.py index c2a1333a224..3896587c068 100644 --- a/py/server/deephaven/table_listener.py +++ b/py/server/deephaven/table_listener.py @@ -22,7 +22,6 @@ from deephaven.table import Table from deephaven.update_graph import UpdateGraph -_JPythonListenerAdapter = jpy.get_type("io.deephaven.integrations.python.PythonListenerAdapter") _JPythonReplayListenerAdapter = jpy.get_type("io.deephaven.integrations.python.PythonReplayListenerAdapter") _JTableUpdate = jpy.get_type("io.deephaven.engine.table.TableUpdate") _JTableUpdateDataReader = jpy.get_type("io.deephaven.integrations.python.PythonListenerTableUpdateDataReader") @@ -67,6 +66,7 @@ class TableUpdate(JObjectWrapper): j_object_type = _JTableUpdate def __init__(self, table: Table, j_table_update: jpy.JType): + super().__init__(j_table_update) self.table = table self.j_table_update = j_table_update @@ -335,8 +335,9 @@ def listen(t: Table, listener: Union[Callable, TableListener], description: str return table_listener_handle -class TableListenerHandle: +class TableListenerHandle:#(JObjectWrapper): """A handle to manage a table listener's lifecycle.""" + # j_object_type = _JPythonReplayListenerAdapter def __init__(self, t: Table, listener: Union[Callable, TableListener], description: str = None): """Creates a new table listener handle. @@ -369,6 +370,7 @@ def __init__(self, t: Table, listener: Union[Callable, TableListener], descripti else: raise ValueError("listener is neither callable nor TableListener object") self.listener = _JPythonReplayListenerAdapter(description, t.j_table, False, listener_wrapped) + # super().__init__(self.listener) self.started = False @@ -408,3 +410,7 @@ def stop(self) -> None: return self.t.j_table.removeUpdateListener(self.listener) self.started = False + + # @property + # def j_object(self) -> jpy.JType: + # return self.listener \ No newline at end of file From 6276e80efd1e996418c49d2f08336f6640b36fe0 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 12 Sep 2023 21:07:37 -0500 Subject: [PATCH 02/18] Draft that looks more like java's api --- py/server/deephaven/liveness_scope.py | 77 +++++++++++++++++++++++---- 1 file changed, 68 insertions(+), 9 deletions(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 5e6dd4d98dc..f27d56c6135 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -8,11 +8,42 @@ import jpy +from typing import Union +from warnings import warn + from deephaven import DHError from deephaven._wrapper import JObjectWrapper _JLivenessScopeStack = jpy.get_type("io.deephaven.engine.liveness.LivenessScopeStack") _JLivenessScope = jpy.get_type("io.deephaven.engine.liveness.LivenessScope") +_JLivenessReferent = jpy.get_type("io.deephaven.engine.liveness.LivenessReferent") + + +def _push(scope: _JLivenessScope): + _JLivenessScopeStack.push(scope) + + +def _pop(scope: _JLivenessScope): + try: + _JLivenessScopeStack.pop(scope) + except Exception as e: + raise DHError(e, message="failed to pop the LivenessScope from the stack.") + + +class LivenessScopeFrame: + """Helper class to support pushing a liveness scope without forcing it to be closed when finished.""" + def __init__(self, scope: "deephaven.liveness_scope.LivenessScope", close_after_block: bool): + self.close_after_pop = close_after_block + self.scope = scope + + def __enter__(self): + _push(self.scope.j_scope) + return self.scope + + def __exit__(self, exc_type, exc_val, exc_tb): + _pop(self.scope.j_scope) + if self.close_after_pop: + self.scope.close() class LivenessScope(JObjectWrapper): @@ -28,10 +59,24 @@ def __init__(self, j_scope: jpy.JType): self.j_scope = j_scope def __enter__(self): + _push(self.j_scope) return self def __exit__(self, exc_type, exc_val, exc_tb): - self.close() + _pop(self.j_scope) + self.release() + + def release(self): + """Closes the LivenessScope and releases all the query graph resources. + + Raises: + DHError + """ + try: + self.j_scope.release() + self.j_scope = None + except Exception as e: + raise DHError(e, message="failed to close the LivenessScope.") def close(self): """Closes the LivenessScope and releases all the query graph resources. @@ -39,13 +84,14 @@ def close(self): Raises: DHError """ - if self.j_scope: - try: - _JLivenessScopeStack.pop(self.j_scope) - self.j_scope.release() - self.j_scope = None - except Exception as e: - raise DHError(e, message="failed to close the LivenessScope.") + warn('This function is deprecated, prefer release()', DeprecationWarning, stacklevel=2) + _pop(self.j_scope) + self.release() + + def open(self, close_after_block: bool = False) -> LivenessScopeFrame: + """Uses this scope for the duration of the `with` block. The scope will not be + closed when the block ends.""" + return LivenessScopeFrame(self, close_after_block) @property def j_object(self) -> jpy.JType: @@ -67,6 +113,20 @@ def preserve(self, wrapper: JObjectWrapper) -> None: except Exception as e: raise DHError(e, message="failed to preserve a wrapped object in this LivenessScope.") + def manage(self, referent: Union[JObjectWrapper, jpy.JType]): + """Explicitly manage the given java object in this scope""" + if isinstance(referent, jpy.JType) and JLivenessReferent.jclass.isInstance(referent): + self.j_scope.manage(referent) + if isinstance(referent, JObjectWrapper): + self.manage(referent.j_object) + + def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]): + """Explicitly unmanage the given java object from this scope""" + if isinstance(referent, jpy.JType) and JLivenessReferent.jclass.isInstance(referent): + self.j_scope.unmanage(referent) + if isinstance(referent, JObjectWrapper): + self.unmanage(referent.j_object) + def liveness_scope() -> LivenessScope: """Creates a LivenessScope for running a block of code. @@ -75,5 +135,4 @@ def liveness_scope() -> LivenessScope: a LivenessScope """ j_scope = _JLivenessScope() - _JLivenessScopeStack.push(j_scope) return LivenessScope(j_scope=j_scope) From 81eeae07d410b3c4c2b11b311960055f91a94d4e Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 14 Sep 2023 09:07:09 -0500 Subject: [PATCH 03/18] Update docs, make wrapper usage more consistent --- py/server/deephaven/liveness_scope.py | 30 +++++++++++++++------------ 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index f27d56c6135..fe2ce9b07f0 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -30,6 +30,13 @@ def _pop(scope: _JLivenessScope): raise DHError(e, message="failed to pop the LivenessScope from the stack.") +def _unwrap_to_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> jpy.JType: + if isinstance(referent, jpy.JType) and _JLivenessReferent.jclass.isInstance(referent): + return referent + if isinstance(referent, JObjectWrapper): + _unwrap_to_liveness_referent(referent.j_object) + raise DHError("Provided referent isn't a LivenessReferent or a JObjectWrapper around one") + class LivenessScopeFrame: """Helper class to support pushing a liveness scope without forcing it to be closed when finished.""" def __init__(self, scope: "deephaven.liveness_scope.LivenessScope", close_after_block: bool): @@ -97,35 +104,32 @@ def open(self, close_after_block: bool = False) -> LivenessScopeFrame: def j_object(self) -> jpy.JType: return self.j_scope - def preserve(self, wrapper: JObjectWrapper) -> None: + def preserve(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: """Preserves a query graph node (usually a Table) to keep it live for the outer scope. Args: - wrapper (JObjectWrapper): a wrapped Java object such as a Table + referent (Union[JObjectWrapper, jpy.JType]): an object to preserve in the next outer liveness scope Raises: DHError """ + referent = _unwrap_to_liveness_referent(referent) try: _JLivenessScopeStack.pop(self.j_scope) - _JLivenessScopeStack.peek().manage(wrapper.j_object) + _JLivenessScopeStack.peek().manage(_unwrap_to_liveness_referent(referent)) _JLivenessScopeStack.push(self.j_scope) except Exception as e: raise DHError(e, message="failed to preserve a wrapped object in this LivenessScope.") def manage(self, referent: Union[JObjectWrapper, jpy.JType]): - """Explicitly manage the given java object in this scope""" - if isinstance(referent, jpy.JType) and JLivenessReferent.jclass.isInstance(referent): - self.j_scope.manage(referent) - if isinstance(referent, JObjectWrapper): - self.manage(referent.j_object) + """Explicitly manage the given java object in this scope. Must only be passed a Java LivenessReferent, or + a Python wrapper around a LivenessReferent""" + self.j_scope.manage(_unwrap_to_liveness_referent(referent)) def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]): - """Explicitly unmanage the given java object from this scope""" - if isinstance(referent, jpy.JType) and JLivenessReferent.jclass.isInstance(referent): - self.j_scope.unmanage(referent) - if isinstance(referent, JObjectWrapper): - self.unmanage(referent.j_object) + """Explicitly unmanage the given java object from this scope. Must only be passed a Java LivenessReferent, or + a Python wrapper around a LivenessReferent""" + self.j_scope.unmanage(_unwrap_to_liveness_referent(referent)) def liveness_scope() -> LivenessScope: From 7b1ee54b5dba87c48e8ade5935e58ba1c34c5ea0 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 14 Sep 2023 13:49:09 -0500 Subject: [PATCH 04/18] Tidy up for review --- py/server/deephaven/_wrapper.py | 16 --------- py/server/deephaven/execution_context.py | 1 - py/server/deephaven/liveness_scope.py | 42 ++++++++++++++++++------ py/server/deephaven/table.py | 1 - py/server/deephaven/table_listener.py | 12 +++---- 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/py/server/deephaven/_wrapper.py b/py/server/deephaven/_wrapper.py index c3212114410..47ed18dd019 100644 --- a/py/server/deephaven/_wrapper.py +++ b/py/server/deephaven/_wrapper.py @@ -22,11 +22,6 @@ _has_all_wrappers_imported = False JLivePyObjectWrapper = jpy.get_type('io.deephaven.server.plugin.python.LivePyObjectWrapper') -JLivenessScope = jpy.get_type("io.deephaven.engine.liveness.LivenessScope") -JLivenessReferent = jpy.get_type('io.deephaven.engine.liveness.LivenessReferent') - - -_interpreter_liveness_scope = JLivenessScope() def _recursive_import(package_path: str) -> None: @@ -58,17 +53,6 @@ def __init_subclass__(cls, *args, **kwargs): if _is_direct_initialisable(cls): _di_wrapper_classes.add(cls) - def __init__(self, j_object: jpy.JType): - self._j_obj = j_object - if JLivenessReferent.jclass.isInstance(self._j_obj): - print('retaining ', self._j_obj) - _interpreter_liveness_scope.tryManage(self._j_obj) - - def __del__(self): - if JLivenessReferent.jclass.isInstance(self._j_obj): - print("releasing ", self._j_obj) - _interpreter_liveness_scope.tryUnmanage(self._j_obj) - @property @abstractmethod def j_object(self) -> jpy.JType: diff --git a/py/server/deephaven/execution_context.py b/py/server/deephaven/execution_context.py index 2a020b6491d..8e416b9e229 100644 --- a/py/server/deephaven/execution_context.py +++ b/py/server/deephaven/execution_context.py @@ -42,7 +42,6 @@ def update_graph(self) -> UpdateGraph: return UpdateGraph(j_update_graph=self.j_exec_ctx.getUpdateGraph()) def __init__(self, j_exec_ctx): - super().__init__(j_exec_ctx) self.j_exec_ctx = j_exec_ctx def __enter__(self): diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index fe2ce9b07f0..02be9b15a14 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -4,7 +4,6 @@ """This module gives the users a finer degree of control over when to clean up unreferenced nodes in the query update graph instead of solely relying on garbage collection.""" -import contextlib import jpy @@ -50,7 +49,7 @@ def __enter__(self): def __exit__(self, exc_type, exc_val, exc_tb): _pop(self.scope.j_scope) if self.close_after_pop: - self.scope.close() + self.scope.release() class LivenessScope(JObjectWrapper): @@ -62,10 +61,11 @@ class LivenessScope(JObjectWrapper): j_object_type = _JLivenessScope def __init__(self, j_scope: jpy.JType): - super().__init__(j_scope) self.j_scope = j_scope def __enter__(self): + warn('Instead of passing liveness_scope() to with, call open() on it first', + DeprecationWarning, stacklevel=2) _push(self.j_scope) return self @@ -91,11 +91,12 @@ def close(self): Raises: DHError """ - warn('This function is deprecated, prefer release()', DeprecationWarning, stacklevel=2) + warn('This function is deprecated, prefer release(). Use cases that rely on this are likely to now fail.', + DeprecationWarning, stacklevel=2) _pop(self.j_scope) self.release() - def open(self, close_after_block: bool = False) -> LivenessScopeFrame: + def open(self, close_after_block: bool = True) -> LivenessScopeFrame: """Uses this scope for the duration of the `with` block. The scope will not be closed when the block ends.""" return LivenessScopeFrame(self, close_after_block) @@ -117,21 +118,42 @@ def preserve(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: try: _JLivenessScopeStack.pop(self.j_scope) _JLivenessScopeStack.peek().manage(_unwrap_to_liveness_referent(referent)) - _JLivenessScopeStack.push(self.j_scope) except Exception as e: raise DHError(e, message="failed to preserve a wrapped object in this LivenessScope.") + finally: + _JLivenessScopeStack.push(self.j_scope) def manage(self, referent: Union[JObjectWrapper, jpy.JType]): - """Explicitly manage the given java object in this scope. Must only be passed a Java LivenessReferent, or - a Python wrapper around a LivenessReferent""" + """ + Explicitly manage the given java object in this scope. Must only be passed a Java LivenessReferent, or + a Python wrapper around a LivenessReferent + """ self.j_scope.manage(_unwrap_to_liveness_referent(referent)) def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]): - """Explicitly unmanage the given java object from this scope. Must only be passed a Java LivenessReferent, or - a Python wrapper around a LivenessReferent""" + """ + Explicitly unmanage the given java object from this scope. Must only be passed a Java LivenessReferent, or + a Python wrapper around a LivenessReferent + """ self.j_scope.unmanage(_unwrap_to_liveness_referent(referent)) +def is_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> bool: + """ + Returns True if the provided object is a LivenessReferent, and so can be managed by a LivenessScope. + Args: + referent: the object that maybe a LivenessReferent + + Returns: + True if the object is a LivenessReferent, False otherwise. + """ + if isinstance(referent, jpy.JType) and _JLivenessReferent.jclass.isInstance(referent): + return True + if isinstance(referent, JObjectWrapper): + return is_liveness_referent(referent.j_object) + return False + + def liveness_scope() -> LivenessScope: """Creates a LivenessScope for running a block of code. diff --git a/py/server/deephaven/table.py b/py/server/deephaven/table.py index 62a4d99a6f0..15181e87691 100644 --- a/py/server/deephaven/table.py +++ b/py/server/deephaven/table.py @@ -507,7 +507,6 @@ class Table(JObjectWrapper): j_object_type = _J_Table def __init__(self, j_table: jpy.JType): - super().__init__(j_table) self.j_table = jpy.cast(j_table, self.j_object_type) if self.j_table is None: raise DHError("j_table type is not io.deephaven.engine.table.Table") diff --git a/py/server/deephaven/table_listener.py b/py/server/deephaven/table_listener.py index 3896587c068..c008d59ce05 100644 --- a/py/server/deephaven/table_listener.py +++ b/py/server/deephaven/table_listener.py @@ -66,7 +66,6 @@ class TableUpdate(JObjectWrapper): j_object_type = _JTableUpdate def __init__(self, table: Table, j_table_update: jpy.JType): - super().__init__(j_table_update) self.table = table self.j_table_update = j_table_update @@ -335,9 +334,9 @@ def listen(t: Table, listener: Union[Callable, TableListener], description: str return table_listener_handle -class TableListenerHandle:#(JObjectWrapper): +class TableListenerHandle(JObjectWrapper): """A handle to manage a table listener's lifecycle.""" - # j_object_type = _JPythonReplayListenerAdapter + j_object_type = _JPythonReplayListenerAdapter def __init__(self, t: Table, listener: Union[Callable, TableListener], description: str = None): """Creates a new table listener handle. @@ -370,7 +369,6 @@ def __init__(self, t: Table, listener: Union[Callable, TableListener], descripti else: raise ValueError("listener is neither callable nor TableListener object") self.listener = _JPythonReplayListenerAdapter(description, t.j_table, False, listener_wrapped) - # super().__init__(self.listener) self.started = False @@ -411,6 +409,6 @@ def stop(self) -> None: self.t.j_table.removeUpdateListener(self.listener) self.started = False - # @property - # def j_object(self) -> jpy.JType: - # return self.listener \ No newline at end of file + @property + def j_object(self) -> jpy.JType: + return self.listener \ No newline at end of file From 0b252f24502d3533254a4fe5d83ee200e7adac37 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 14 Sep 2023 15:27:36 -0500 Subject: [PATCH 05/18] Self-review before PR --- py/server/deephaven/liveness_scope.py | 74 ++++++++++++++++++--------- py/server/deephaven/table_listener.py | 2 +- 2 files changed, 52 insertions(+), 24 deletions(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 02be9b15a14..321909ff3f4 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -4,6 +4,7 @@ """This module gives the users a finer degree of control over when to clean up unreferenced nodes in the query update graph instead of solely relying on garbage collection.""" +import contextlib import jpy @@ -36,21 +37,6 @@ def _unwrap_to_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> _unwrap_to_liveness_referent(referent.j_object) raise DHError("Provided referent isn't a LivenessReferent or a JObjectWrapper around one") -class LivenessScopeFrame: - """Helper class to support pushing a liveness scope without forcing it to be closed when finished.""" - def __init__(self, scope: "deephaven.liveness_scope.LivenessScope", close_after_block: bool): - self.close_after_pop = close_after_block - self.scope = scope - - def __enter__(self): - _push(self.scope.j_scope) - return self.scope - - def __exit__(self, exc_type, exc_val, exc_tb): - _pop(self.scope.j_scope) - if self.close_after_pop: - self.scope.release() - class LivenessScope(JObjectWrapper): """A LivenessScope automatically manages reference counting of tables and other query resources that are @@ -96,10 +82,26 @@ def close(self): _pop(self.j_scope) self.release() - def open(self, close_after_block: bool = True) -> LivenessScopeFrame: - """Uses this scope for the duration of the `with` block. The scope will not be - closed when the block ends.""" - return LivenessScopeFrame(self, close_after_block) + @contextlib.contextmanager + def open(self, release_after_block: bool = True) -> None: + """ + Uses this scope for the duration of the `with` block. The scope defaults to being closed + when the block ends, disable by passing release_after_block=False + + Args: + release_after_block: True to release the scope when the block ends, False to leave the + scope open for reuse. + + Returns: None, to allow changes in the future. + + """ + _push(self.j_scope) + try: + yield None + finally: + _pop(self.j_scope) + if release_after_block: + self.release() @property def j_object(self) -> jpy.JType: @@ -123,19 +125,45 @@ def preserve(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: finally: _JLivenessScopeStack.push(self.j_scope) - def manage(self, referent: Union[JObjectWrapper, jpy.JType]): + def manage(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: """ Explicitly manage the given java object in this scope. Must only be passed a Java LivenessReferent, or a Python wrapper around a LivenessReferent + + Args: + referent: the object to manage by this scope + + Returns: None + + Raises: + DHError if the referent isn't a LivenessReferent, or if it is no longer live + """ - self.j_scope.manage(_unwrap_to_liveness_referent(referent)) + referent = _unwrap_to_liveness_referent(referent) + try: + self.j_scope.manage(referent) + except Exception as e: + raise DHError(e, message="failed to manage object") - def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]): + def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: """ Explicitly unmanage the given java object from this scope. Must only be passed a Java LivenessReferent, or a Python wrapper around a LivenessReferent + + Args: + referent: + + Returns: None + + Raises: + DHError if the referent isn't a LivenessReferent, or if it is no longer live + """ - self.j_scope.unmanage(_unwrap_to_liveness_referent(referent)) + referent = _unwrap_to_liveness_referent(referent) + try: + self.j_scope.unmanage(referent) + except Exception as e: + raise DHError(e, message="failed to unmanage object") def is_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> bool: diff --git a/py/server/deephaven/table_listener.py b/py/server/deephaven/table_listener.py index c008d59ce05..faa874c0b3e 100644 --- a/py/server/deephaven/table_listener.py +++ b/py/server/deephaven/table_listener.py @@ -411,4 +411,4 @@ def stop(self) -> None: @property def j_object(self) -> jpy.JType: - return self.listener \ No newline at end of file + return self.listener From 13dd0df6d41407aedfc1637b5789be7bdc96a006 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 18 Sep 2023 09:37:20 -0500 Subject: [PATCH 06/18] Update py/server/deephaven/liveness_scope.py --- py/server/deephaven/liveness_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 321909ff3f4..9037a06098d 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -151,7 +151,7 @@ def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: a Python wrapper around a LivenessReferent Args: - referent: + referent: the object to unmanage from this scope Returns: None From 9721921c3f4924749422b0e9e5f9cfa4682216a4 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 18 Sep 2023 18:42:45 -0500 Subject: [PATCH 07/18] Draft at fixing py api, tests --- py/server/deephaven/liveness_scope.py | 13 +++-- py/server/tests/test_liveness.py | 75 +++++++++++++++++++++++---- py/server/tests/testbase.py | 4 +- 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 9037a06098d..5a3e8cd6688 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -34,7 +34,7 @@ def _unwrap_to_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> if isinstance(referent, jpy.JType) and _JLivenessReferent.jclass.isInstance(referent): return referent if isinstance(referent, JObjectWrapper): - _unwrap_to_liveness_referent(referent.j_object) + return _unwrap_to_liveness_referent(referent.j_object) raise DHError("Provided referent isn't a LivenessReferent or a JObjectWrapper around one") @@ -83,7 +83,7 @@ def close(self): self.release() @contextlib.contextmanager - def open(self, release_after_block: bool = True) -> None: + def open(self, release_after_block: bool = True) -> "LivenessScope": """ Uses this scope for the duration of the `with` block. The scope defaults to being closed when the block ends, disable by passing release_after_block=False @@ -97,7 +97,7 @@ def open(self, release_after_block: bool = True) -> None: """ _push(self.j_scope) try: - yield None + yield self finally: _pop(self.j_scope) if release_after_block: @@ -118,11 +118,18 @@ def preserve(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: """ referent = _unwrap_to_liveness_referent(referent) try: + # Ensure we are the current scope, throw DHError if we aren't _JLivenessScopeStack.pop(self.j_scope) + except Exception as e: + raise DHError(e, message="failed to pop the current scope - is preserve() being called on the right scope?") + + try: + # Manage the object in the next outer scope on this thread. _JLivenessScopeStack.peek().manage(_unwrap_to_liveness_referent(referent)) except Exception as e: raise DHError(e, message="failed to preserve a wrapped object in this LivenessScope.") finally: + # Success or failure, restore the scope that was successfully popped _JLivenessScopeStack.push(self.j_scope) def manage(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: diff --git a/py/server/tests/test_liveness.py b/py/server/tests/test_liveness.py index f6d2203345b..e78e506012a 100644 --- a/py/server/tests/test_liveness.py +++ b/py/server/tests/test_liveness.py @@ -24,7 +24,10 @@ def create_table(self): with exclusive_lock(self.test_update_graph): return time_table("PT00:00:00.001").update(["X=i%11"]).sort("X").tail(16) - def test_liveness(self): + def test_deprecated_liveness(self): + # Old test code, to make sure the basic cases still behave. Complex cases didn't make + # sense before, so aren't heavily tested. Note that these will emit warnings in the + # test output. not_managed = self.create_table() with liveness_scope() as l_scope: to_discard = self.create_table() @@ -43,28 +46,51 @@ def test_liveness(self): must_keep = self.create_table() df = to_pandas(must_keep) - with self.assertRaises(DHError): - l_scope = liveness_scope() + def test_liveness(self): + not_managed = self.create_table() + with liveness_scope().open() as l_scope: to_discard = self.create_table() df = to_pandas(to_discard) - l_scope_2 = liveness_scope() must_keep = self.create_table() df = to_pandas(must_keep) l_scope.preserve(must_keep) - l_scope.close() - l_scope_2.close() - l_scope_2.close() - l_scope.close() + + self.assertTrue(not_managed.j_table.tryRetainReference()) + self.assertTrue(must_keep.j_table.tryRetainReference()) + self.assertFalse(to_discard.j_table.tryRetainReference()) + + with liveness_scope().open(): + to_discard = self.create_table() + df = to_pandas(to_discard) + must_keep = self.create_table() + df = to_pandas(must_keep) + + with self.assertRaises(DHError): + l_scope = liveness_scope() + with l_scope.open(False): + to_discard = self.create_table() + df = to_pandas(to_discard) + l_scope_2 = liveness_scope() + with l_scope_2.open(False): + must_keep = self.create_table() + df = to_pandas(must_keep) + l_scope.preserve(must_keep) # throws DHError + l_scope.release() # will never run + l_scope_2.release() # will never run + l_scope_2.release() + l_scope.release() def test_liveness_nested(self): - with liveness_scope() as l_scope: + l_scope = liveness_scope() + with l_scope.open(): to_discard = self.create_table() df = to_pandas(to_discard) must_keep = self.create_table() df = to_pandas(must_keep) l_scope.preserve(must_keep) - with liveness_scope() as nested_l_scope: + nested_l_scope = liveness_scope() + with nested_l_scope.open(): nested_to_discard = self.create_table() df = to_pandas(nested_to_discard) nested_must_keep = self.create_table() @@ -80,6 +106,35 @@ def test_liveness_nested(self): self.assertFalse(nested_must_keep.j_table.tryRetainReference()) self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + def test_liveness_siblings(self): + l_scope = liveness_scope() + with l_scope.open(): + to_discard = self.create_table() + df = to_pandas(to_discard) + must_keep = self.create_table() + df = to_pandas(must_keep) + l_scope.preserve(must_keep) + + # Create a second scope and interact with it + other_l_scope = liveness_scope() + with other_l_scope.open(): + nested_to_discard = self.create_table() + df = to_pandas(nested_to_discard) + nested_must_keep = self.create_table() + df = to_pandas(nested_must_keep) + other_l_scope.preserve(nested_must_keep) + self.assertTrue(nested_must_keep.j_table.tryRetainReference()) + # drop the extra reference obtained by the tryRetainReference() call in the above assert + nested_must_keep.j_table.dropReference() + self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + + + + self.assertTrue(must_keep.j_table.tryRetainReference()) + self.assertFalse(to_discard.j_table.tryRetainReference()) + self.assertFalse(nested_must_keep.j_table.tryRetainReference()) + self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + if __name__ == '__main__': unittest.main() diff --git a/py/server/tests/testbase.py b/py/server/tests/testbase.py index 57caecfc4fc..0aa51cac616 100644 --- a/py/server/tests/testbase.py +++ b/py/server/tests/testbase.py @@ -35,9 +35,11 @@ def tearDownClass(cls) -> None: def setUp(self) -> None: self._liveness_scope = liveness_scope() + self.opened_scope = liveness_scope().open() def tearDown(self) -> None: - self._liveness_scope.close() + with self.opened_scope: + ... def wait_ticking_table_update(self, table: Table, row_count: int, timeout: int): """Waits for a ticking table to grow to the specified size or times out. From 32dd8819a541dec861dbc3a4ec2595a666e7694d Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 19 Sep 2023 11:40:00 -0500 Subject: [PATCH 08/18] Cleanup docs --- py/server/deephaven/liveness_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 5a3e8cd6688..a356e22706f 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -90,7 +90,7 @@ def open(self, release_after_block: bool = True) -> "LivenessScope": Args: release_after_block: True to release the scope when the block ends, False to leave the - scope open for reuse. + scope open, allowing it to be reused and keeping collected referents live. Returns: None, to allow changes in the future. From 3079dde286c59454d46e089fdc070fa05364ad4b Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 20 Sep 2023 10:23:35 -0500 Subject: [PATCH 09/18] Update py/server/deephaven/liveness_scope.py Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- py/server/deephaven/liveness_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index a356e22706f..77a6464e847 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -90,7 +90,7 @@ def open(self, release_after_block: bool = True) -> "LivenessScope": Args: release_after_block: True to release the scope when the block ends, False to leave the - scope open, allowing it to be reused and keeping collected referents live. + scope open, allowing it to be reused and keeping collected referents live. Default is True. Returns: None, to allow changes in the future. From 7a944034278eb4f3b361558149d7b516d27f2210 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 20 Sep 2023 10:46:11 -0500 Subject: [PATCH 10/18] Update py/server/deephaven/liveness_scope.py Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- py/server/deephaven/liveness_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 77a6464e847..00d4ec47c4a 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -59,7 +59,7 @@ def __exit__(self, exc_type, exc_val, exc_tb): _pop(self.j_scope) self.release() - def release(self): + def release(self) -> None: """Closes the LivenessScope and releases all the query graph resources. Raises: From d0e8c83c96386b85b533d1a803bd0d0f0ae63070 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 20 Sep 2023 13:44:13 -0500 Subject: [PATCH 11/18] Enhance docs --- py/server/deephaven/liveness_scope.py | 13 ++++++------- py/server/tests/testbase.py | 1 - 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 00d4ec47c4a..b29439cac59 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -63,7 +63,7 @@ def release(self) -> None: """Closes the LivenessScope and releases all the query graph resources. Raises: - DHError + DHError if this instance has been released too many times """ try: self.j_scope.release() @@ -75,7 +75,7 @@ def close(self): """Closes the LivenessScope and releases all the query graph resources. Raises: - DHError + DHError if this instance wasn't on the top of the stack, or this instance was released too many times """ warn('This function is deprecated, prefer release(). Use cases that rely on this are likely to now fail.', DeprecationWarning, stacklevel=2) @@ -89,11 +89,10 @@ def open(self, release_after_block: bool = True) -> "LivenessScope": when the block ends, disable by passing release_after_block=False Args: - release_after_block: True to release the scope when the block ends, False to leave the + release_after_block (bool): True to release the scope when the block ends, False to leave the scope open, allowing it to be reused and keeping collected referents live. Default is True. Returns: None, to allow changes in the future. - """ _push(self.j_scope) try: @@ -114,7 +113,7 @@ def preserve(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: referent (Union[JObjectWrapper, jpy.JType]): an object to preserve in the next outer liveness scope Raises: - DHError + DHError if the object isn't a liveness node, or this instance isn't currently at the stop of the stack """ referent = _unwrap_to_liveness_referent(referent) try: @@ -138,7 +137,7 @@ def manage(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: a Python wrapper around a LivenessReferent Args: - referent: the object to manage by this scope + referent (Union[JObjectWrapper, jpy.JType]): the object to manage by this scope Returns: None @@ -158,7 +157,7 @@ def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: a Python wrapper around a LivenessReferent Args: - referent: the object to unmanage from this scope + referent (Union[JObjectWrapper, jpy.JType]): the object to unmanage from this scope Returns: None diff --git a/py/server/tests/testbase.py b/py/server/tests/testbase.py index 0aa51cac616..89d928cd42c 100644 --- a/py/server/tests/testbase.py +++ b/py/server/tests/testbase.py @@ -34,7 +34,6 @@ def tearDownClass(cls) -> None: cls._execution_context.close() def setUp(self) -> None: - self._liveness_scope = liveness_scope() self.opened_scope = liveness_scope().open() def tearDown(self) -> None: From 5a800b10681ea0a4b3fe542fb6b522be07f16095 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 21 Sep 2023 08:15:44 -0500 Subject: [PATCH 12/18] Improve tests --- py/server/tests/test_liveness.py | 53 +++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/py/server/tests/test_liveness.py b/py/server/tests/test_liveness.py index e78e506012a..cd5480e4c8b 100644 --- a/py/server/tests/test_liveness.py +++ b/py/server/tests/test_liveness.py @@ -123,17 +123,62 @@ def test_liveness_siblings(self): nested_must_keep = self.create_table() df = to_pandas(nested_must_keep) other_l_scope.preserve(nested_must_keep) + + self.assertFalse(to_discard.j_table.tryRetainReference()) + self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + self.assertTrue(nested_must_keep.j_table.tryRetainReference()) - # drop the extra reference obtained by the tryRetainReference() call in the above assert + # drop extra reference nested_must_keep.j_table.dropReference() - self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + self.assertTrue(must_keep.j_table.tryRetainReference()) + # drop extra reference + must_keep.j_table.dropReference() + + self.assertFalse(to_discard.j_table.tryRetainReference()) + def test_reopen_scope(self): + l_scope = liveness_scope() + with l_scope.open(False): + to_discard = self.create_table() + df = to_pandas(to_discard) + must_keep = self.create_table() + df = to_pandas(must_keep) + l_scope.preserve(must_keep) + + self.assertTrue(to_discard.j_table.tryRetainReference()) + # drop extra reference + to_discard.j_table.dropReference() self.assertTrue(must_keep.j_table.tryRetainReference()) + # drop extra reference + must_keep.j_table.dropReference() + + # Reopen the scope and add to it again + with l_scope.open(False): + to_discard_2 = self.create_table() + df = to_pandas(to_discard_2) + + self.assertTrue(to_discard.j_table.tryRetainReference()) + # drop extra reference + to_discard.j_table.dropReference() + + self.assertTrue(to_discard_2.j_table.tryRetainReference()) + # drop extra reference + to_discard_2.j_table.dropReference() + + self.assertTrue(must_keep.j_table.tryRetainReference()) + # drop extra reference + must_keep.j_table.dropReference() + + l_scope.release() + self.assertFalse(to_discard.j_table.tryRetainReference()) - self.assertFalse(nested_must_keep.j_table.tryRetainReference()) - self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + self.assertFalse(to_discard_2.j_table.tryRetainReference()) + + self.assertTrue(must_keep.j_table.tryRetainReference()) + # drop extra reference + must_keep.j_table.dropReference() if __name__ == '__main__': From fdf1ca4c1119503b16cbe3ea31b4933c8637dd6a Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 29 Sep 2023 09:05:04 -0500 Subject: [PATCH 13/18] Updated api, docs to match new discussed patterns --- py/server/deephaven/liveness_scope.py | 175 +++++++++++++++++--------- py/server/tests/test_liveness.py | 59 ++++++--- py/server/tests/test_perfmon.py | 1 - py/server/tests/testbase.py | 4 +- sphinx/source/conf.py | 1 + 5 files changed, 158 insertions(+), 82 deletions(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index b29439cac59..80bca82e219 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -3,7 +3,54 @@ # """This module gives the users a finer degree of control over when to clean up unreferenced nodes in the query update -graph instead of solely relying on garbage collection.""" +graph instead of solely relying on garbage collection. + +Examples: + + Use the liveness_scope() function to produce a simple liveness scope that will be used only within a `with` expression + or as a decorator. + + .. code-block:: python + + with liveness_scope() as scope: + ticking_table = some_ticking_source() + table = ticking_table.snapshot().join(table=other_ticking_table, on=...) + scope.preserve(table) + return table + + .. code-block:: python + + @liveness_scope + def get_values(): + ticking_table = some_ticking_source().last_by("Sym") + return dhnp.to_numpy(ticking_table) + + + Use the LivenessScope type for greater control, allowing a scope to be opened more than once, and to release the + resources that it manages when the scope will no longer be used. + + + .. code-block:: python + + def make_table_and_scope(a: int): + scope = LivenessScope() + with scope.open(): + ticking_table = some_ticking_source().where(f"A={a}") + return some_ticking_table, scope + + t1, s1 = make_table_and_scope(1) + # .. wait for a while + s1.release() + t2, s2 = make_table_and_scope(2) + # etc + + In both cases, the scope object has a few methods that can be used to more directly manage liveness referents: + * `scope.preserve(obj)` will preserve the given instance in the next scope outside the specified scope instance + * `scope.manange(obj)` will directly manage the given instance in the specified scope. Take care not to + double-manage objects when using in conjunction with a `with` block or function decorator. + * `scope.unmanage(obj)` will stop managing the given instance. This can be used regardless of how the instance + was managed to begin with. +""" import contextlib import jpy @@ -38,28 +85,16 @@ def _unwrap_to_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> raise DHError("Provided referent isn't a LivenessReferent or a JObjectWrapper around one") -class LivenessScope(JObjectWrapper): - """A LivenessScope automatically manages reference counting of tables and other query resources that are - created in it. It implements the context manager protocol and thus can be used in the with statement. - - Note, LivenessScope should not be instantiated directly but rather through the 'liveness_scope' function. +class _BaseLivenessScope(JObjectWrapper): + """ + Internal base type for Java LivenessScope types in python. """ j_object_type = _JLivenessScope - def __init__(self, j_scope: jpy.JType): - self.j_scope = j_scope - - def __enter__(self): - warn('Instead of passing liveness_scope() to with, call open() on it first', - DeprecationWarning, stacklevel=2) - _push(self.j_scope) - return self - - def __exit__(self, exc_type, exc_val, exc_tb): - _pop(self.j_scope) - self.release() + def __init__(self): + self.j_scope = _JLivenessScope() - def release(self) -> None: + def _release(self) -> None: """Closes the LivenessScope and releases all the query graph resources. Raises: @@ -71,41 +106,6 @@ def release(self) -> None: except Exception as e: raise DHError(e, message="failed to close the LivenessScope.") - def close(self): - """Closes the LivenessScope and releases all the query graph resources. - - Raises: - DHError if this instance wasn't on the top of the stack, or this instance was released too many times - """ - warn('This function is deprecated, prefer release(). Use cases that rely on this are likely to now fail.', - DeprecationWarning, stacklevel=2) - _pop(self.j_scope) - self.release() - - @contextlib.contextmanager - def open(self, release_after_block: bool = True) -> "LivenessScope": - """ - Uses this scope for the duration of the `with` block. The scope defaults to being closed - when the block ends, disable by passing release_after_block=False - - Args: - release_after_block (bool): True to release the scope when the block ends, False to leave the - scope open, allowing it to be reused and keeping collected referents live. Default is True. - - Returns: None, to allow changes in the future. - """ - _push(self.j_scope) - try: - yield self - finally: - _pop(self.j_scope) - if release_after_block: - self.release() - - @property - def j_object(self) -> jpy.JType: - return self.j_scope - def preserve(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: """Preserves a query graph node (usually a Table) to keep it live for the outer scope. @@ -171,12 +171,55 @@ def unmanage(self, referent: Union[JObjectWrapper, jpy.JType]) -> None: except Exception as e: raise DHError(e, message="failed to unmanage object") + @property + def j_object(self) -> jpy.JType: + return self.j_scope + + +class SimpleLivenessScope(_BaseLivenessScope): + """ + A SimpleLivenessScope automatically managed reference counting of tables and other query resources that + are created in it. Instances are created through calling `liveness_scope()` in a `with` block or as a + function decorator, and will be disposed of automatically. + """ + + +class LivenessScope(_BaseLivenessScope): + """A LivenessScope automatically manages reference counting of tables and other query resources that are + created in it. Any created instances must have `release()` called on them to correctly free resources + that have been managed. + """ + + def __init__(self): + super().__init__() + + def release(self) -> None: + """Closes the LivenessScope and releases all the managed resources. + + Raises: + DHError if this instance has been released too many times + """ + self._release() + + @contextlib.contextmanager + def open(self) -> None: + """ + Uses this scope for the duration of a `with` block, automatically managing all resources created in the block. + + Returns: None, to allow changes in the future. + """ + _push(self.j_scope) + try: + yield None + finally: + _pop(self.j_scope) + def is_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> bool: """ Returns True if the provided object is a LivenessReferent, and so can be managed by a LivenessScope. Args: - referent: the object that maybe a LivenessReferent + referent: the object that may be a LivenessReferent Returns: True if the object is a LivenessReferent, False otherwise. @@ -188,11 +231,21 @@ def is_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> bool: return False -def liveness_scope() -> LivenessScope: - """Creates a LivenessScope for running a block of code. +@contextlib.contextmanager +def liveness_scope() -> SimpleLivenessScope: + """Creates and opens a LivenessScope for running a block of code. Use + this function to wrap a block of code using a `with` statement. - Returns: - a LivenessScope + For the duration of the `with` block, the liveness scope will be open + and any liveness referents created will be manged by it automatically. + + Yields: + a SimpleLivenessScope """ - j_scope = _JLivenessScope() - return LivenessScope(j_scope=j_scope) + scope = SimpleLivenessScope() + _push(scope.j_scope) + try: + yield scope + finally: + _pop(scope.j_scope) + scope._release() diff --git a/py/server/tests/test_liveness.py b/py/server/tests/test_liveness.py index cd5480e4c8b..e17f91c6418 100644 --- a/py/server/tests/test_liveness.py +++ b/py/server/tests/test_liveness.py @@ -9,7 +9,7 @@ from deephaven import time_table, DHError from deephaven.execution_context import get_exec_ctx -from deephaven.liveness_scope import liveness_scope +from deephaven.liveness_scope import liveness_scope, LivenessScope from deephaven.update_graph import exclusive_lock from tests.testbase import BaseTestCase @@ -24,7 +24,7 @@ def create_table(self): with exclusive_lock(self.test_update_graph): return time_table("PT00:00:00.001").update(["X=i%11"]).sort("X").tail(16) - def test_deprecated_liveness(self): + def test_simple_liveness(self): # Old test code, to make sure the basic cases still behave. Complex cases didn't make # sense before, so aren't heavily tested. Note that these will emit warnings in the # test output. @@ -46,9 +46,34 @@ def test_deprecated_liveness(self): must_keep = self.create_table() df = to_pandas(must_keep) + def test_simple_liveness_nested(self): + with liveness_scope() as l_scope: + to_discard = self.create_table() + df = to_pandas(to_discard) + must_keep = self.create_table() + df = to_pandas(must_keep) + l_scope.preserve(must_keep) + + with liveness_scope() as nested_l_scope: + nested_to_discard = self.create_table() + df = to_pandas(nested_to_discard) + nested_must_keep = self.create_table() + df = to_pandas(nested_must_keep) + nested_l_scope.preserve(nested_must_keep) + self.assertTrue(nested_must_keep.j_table.tryRetainReference()) + # drop the extra reference obtained by the tryRetainReference() call in the above assert + nested_must_keep.j_table.dropReference() + self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + + self.assertTrue(must_keep.j_table.tryRetainReference()) + must_keep.j_table.dropReference() + self.assertFalse(to_discard.j_table.tryRetainReference()) + self.assertFalse(nested_must_keep.j_table.tryRetainReference()) + self.assertFalse(nested_to_discard.j_table.tryRetainReference()) + def test_liveness(self): not_managed = self.create_table() - with liveness_scope().open() as l_scope: + with liveness_scope() as l_scope: to_discard = self.create_table() df = to_pandas(to_discard) must_keep = self.create_table() @@ -59,19 +84,19 @@ def test_liveness(self): self.assertTrue(must_keep.j_table.tryRetainReference()) self.assertFalse(to_discard.j_table.tryRetainReference()) - with liveness_scope().open(): + with liveness_scope(): to_discard = self.create_table() df = to_pandas(to_discard) must_keep = self.create_table() df = to_pandas(must_keep) with self.assertRaises(DHError): - l_scope = liveness_scope() - with l_scope.open(False): + l_scope = LivenessScope() + with l_scope.open(): to_discard = self.create_table() df = to_pandas(to_discard) - l_scope_2 = liveness_scope() - with l_scope_2.open(False): + l_scope_2 = LivenessScope() + with l_scope_2.open(): must_keep = self.create_table() df = to_pandas(must_keep) l_scope.preserve(must_keep) # throws DHError @@ -81,16 +106,14 @@ def test_liveness(self): l_scope.release() def test_liveness_nested(self): - l_scope = liveness_scope() - with l_scope.open(): + with liveness_scope() as l_scope: to_discard = self.create_table() df = to_pandas(to_discard) must_keep = self.create_table() df = to_pandas(must_keep) l_scope.preserve(must_keep) - nested_l_scope = liveness_scope() - with nested_l_scope.open(): + with liveness_scope() as nested_l_scope: nested_to_discard = self.create_table() df = to_pandas(nested_to_discard) nested_must_keep = self.create_table() @@ -107,8 +130,7 @@ def test_liveness_nested(self): self.assertFalse(nested_to_discard.j_table.tryRetainReference()) def test_liveness_siblings(self): - l_scope = liveness_scope() - with l_scope.open(): + with liveness_scope() as l_scope: to_discard = self.create_table() df = to_pandas(to_discard) must_keep = self.create_table() @@ -116,8 +138,7 @@ def test_liveness_siblings(self): l_scope.preserve(must_keep) # Create a second scope and interact with it - other_l_scope = liveness_scope() - with other_l_scope.open(): + with liveness_scope() as other_l_scope: nested_to_discard = self.create_table() df = to_pandas(nested_to_discard) nested_must_keep = self.create_table() @@ -138,8 +159,8 @@ def test_liveness_siblings(self): self.assertFalse(to_discard.j_table.tryRetainReference()) def test_reopen_scope(self): - l_scope = liveness_scope() - with l_scope.open(False): + l_scope = LivenessScope() + with l_scope.open(): to_discard = self.create_table() df = to_pandas(to_discard) must_keep = self.create_table() @@ -155,7 +176,7 @@ def test_reopen_scope(self): must_keep.j_table.dropReference() # Reopen the scope and add to it again - with l_scope.open(False): + with l_scope.open(): to_discard_2 = self.create_table() df = to_pandas(to_discard_2) diff --git a/py/server/tests/test_perfmon.py b/py/server/tests/test_perfmon.py index ffa63780c96..121b774aea4 100644 --- a/py/server/tests/test_perfmon.py +++ b/py/server/tests/test_perfmon.py @@ -5,7 +5,6 @@ import unittest from deephaven import empty_table -from deephaven.liveness_scope import liveness_scope from deephaven.perfmon import process_info_log, process_metrics_log, server_state_log, \ query_operation_performance_log, query_performance_log, update_performance_log, metrics_get_counters, \ metrics_reset_counters diff --git a/py/server/tests/testbase.py b/py/server/tests/testbase.py index 89d928cd42c..1b63966a0df 100644 --- a/py/server/tests/testbase.py +++ b/py/server/tests/testbase.py @@ -34,7 +34,9 @@ def tearDownClass(cls) -> None: cls._execution_context.close() def setUp(self) -> None: - self.opened_scope = liveness_scope().open() + # Note that this is technically not a supported way to use liveness_scope, but we are deliberately leaving + # the scope open across separate method calls, which we would normally consider unsafe. + self.opened_scope = liveness_scope() def tearDown(self) -> None: with self.opened_scope: diff --git a/sphinx/source/conf.py b/sphinx/source/conf.py index 372dd482159..61ae8ca5b95 100644 --- a/sphinx/source/conf.py +++ b/sphinx/source/conf.py @@ -111,6 +111,7 @@ py_dh_session = _JPythonScriptSession(docs_update_graph, py_scope_jpy) py_dh_session.getExecutionContext().open() +pygments_style = 'sphinx' import deephaven docs_title = "Deephaven python modules." From 91150981c44e3b54bc2a5cc095d33610d7e61d8a Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 6 Oct 2023 13:37:00 -0500 Subject: [PATCH 14/18] Update py/server/deephaven/liveness_scope.py Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- py/server/deephaven/liveness_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 80bca82e219..4b023fa8f13 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -178,7 +178,7 @@ def j_object(self) -> jpy.JType: class SimpleLivenessScope(_BaseLivenessScope): """ - A SimpleLivenessScope automatically managed reference counting of tables and other query resources that + A SimpleLivenessScope automatically manages reference counting of tables and other query resources that are created in it. Instances are created through calling `liveness_scope()` in a `with` block or as a function decorator, and will be disposed of automatically. """ From f1c90ef16a8a1e28c696cd9a185bdbd57e564aa3 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 6 Oct 2023 13:37:12 -0500 Subject: [PATCH 15/18] Update py/server/deephaven/liveness_scope.py Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- py/server/deephaven/liveness_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 4b023fa8f13..e3ec85fbf62 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -70,7 +70,7 @@ def _push(scope: _JLivenessScope): _JLivenessScopeStack.push(scope) -def _pop(scope: _JLivenessScope): +def _pop(scope: _JLivenessScope) -> None: try: _JLivenessScopeStack.pop(scope) except Exception as e: From b8c890a0c78a774d6c01594ea252ffb852930122 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 6 Oct 2023 13:37:23 -0500 Subject: [PATCH 16/18] Update py/server/deephaven/liveness_scope.py Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- py/server/deephaven/liveness_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index e3ec85fbf62..73218faf239 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -66,7 +66,7 @@ def make_table_and_scope(a: int): _JLivenessReferent = jpy.get_type("io.deephaven.engine.liveness.LivenessReferent") -def _push(scope: _JLivenessScope): +def _push(scope: _JLivenessScope) -> None: _JLivenessScopeStack.push(scope) From 7e456b72f20969fae8eea065d69b4432f184d73a Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 6 Oct 2023 13:49:00 -0500 Subject: [PATCH 17/18] Added a test, corrected contextmanager typehints --- py/server/deephaven/liveness_scope.py | 6 +++--- py/server/tests/test_liveness.py | 14 +++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/py/server/deephaven/liveness_scope.py b/py/server/deephaven/liveness_scope.py index 73218faf239..296e99588e8 100644 --- a/py/server/deephaven/liveness_scope.py +++ b/py/server/deephaven/liveness_scope.py @@ -55,7 +55,7 @@ def make_table_and_scope(a: int): import jpy -from typing import Union +from typing import Union, Iterator from warnings import warn from deephaven import DHError @@ -202,7 +202,7 @@ def release(self) -> None: self._release() @contextlib.contextmanager - def open(self) -> None: + def open(self) -> Iterator[None]: """ Uses this scope for the duration of a `with` block, automatically managing all resources created in the block. @@ -232,7 +232,7 @@ def is_liveness_referent(referent: Union[JObjectWrapper, jpy.JType]) -> bool: @contextlib.contextmanager -def liveness_scope() -> SimpleLivenessScope: +def liveness_scope() -> Iterator[SimpleLivenessScope]: """Creates and opens a LivenessScope for running a block of code. Use this function to wrap a block of code using a `with` statement. diff --git a/py/server/tests/test_liveness.py b/py/server/tests/test_liveness.py index e17f91c6418..d088ca55084 100644 --- a/py/server/tests/test_liveness.py +++ b/py/server/tests/test_liveness.py @@ -36,17 +36,21 @@ def test_simple_liveness(self): df = to_pandas(must_keep) l_scope.preserve(must_keep) - self.assertTrue(not_managed.j_table.tryRetainReference()) self.assertTrue(must_keep.j_table.tryRetainReference()) self.assertFalse(to_discard.j_table.tryRetainReference()) - with liveness_scope(): + @liveness_scope() + def function_test(): to_discard = self.create_table() df = to_pandas(to_discard) - must_keep = self.create_table() - df = to_pandas(must_keep) - def test_simple_liveness_nested(self): + function_test() + self.assertFalse(to_discard.j_table.tryRetainReference()) + + self.assertTrue(not_managed.j_table.tryRetainReference()) + + +def test_simple_liveness_nested(self): with liveness_scope() as l_scope: to_discard = self.create_table() df = to_pandas(to_discard) From 5ddb9658c4a298b547881658ec6b3cd0d1c06eaf Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 6 Oct 2023 14:17:06 -0500 Subject: [PATCH 18/18] correct indentation --- py/server/tests/test_liveness.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/py/server/tests/test_liveness.py b/py/server/tests/test_liveness.py index d088ca55084..6b4ab3724fd 100644 --- a/py/server/tests/test_liveness.py +++ b/py/server/tests/test_liveness.py @@ -49,8 +49,7 @@ def function_test(): self.assertTrue(not_managed.j_table.tryRetainReference()) - -def test_simple_liveness_nested(self): + def test_simple_liveness_nested(self): with liveness_scope() as l_scope: to_discard = self.create_table() df = to_pandas(to_discard)