From 62c57c16103882828060c47962f30046583f1230 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 16 Feb 2024 13:26:59 -0600 Subject: [PATCH 1/5] Fix use_memo usage, update use_table_listener to memoize --- docker/data/storage/notebooks/DEMO.md | 16 ++++++++-------- plugins/ui/src/deephaven/ui/hooks/use_effect.py | 3 ++- .../deephaven/ui/hooks/use_execution_context.py | 4 ++-- .../src/deephaven/ui/hooks/use_table_listener.py | 8 ++++++-- plugins/ui/test/deephaven/ui/test_hooks.py | 6 +++--- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/docker/data/storage/notebooks/DEMO.md b/docker/data/storage/notebooks/DEMO.md index 7d7749234..008631a72 100644 --- a/docker/data/storage/notebooks/DEMO.md +++ b/docker/data/storage/notebooks/DEMO.md @@ -187,7 +187,7 @@ def hist_demo(source, column): x=column, nbins=bin_count, ), - [bin_count, hist_range, source, column], + {bin_count, hist_range, source, column}, ) return [ @@ -230,9 +230,9 @@ def order_table(): lambda: table_publisher( "Order table", {"sym": dht.string, "size": dht.int32, "side": dht.string} ), - [], + {}, ) - t = ui.use_memo(lambda: blink_to_append_only(blink_table), [blink_table]) + t = ui.use_memo(lambda: blink_to_append_only(blink_table), {blink_table}) def submit_order(order_sym, order_size, side): publisher.add( @@ -389,7 +389,7 @@ def use_wave_input(): def multiwave(): amplitude, frequency, phase, wave_input = use_wave_input() - tt = use_memo(lambda: time_table("PT1s").update("x=i"), []) + tt = use_memo(lambda: time_table("PT1s").update("x=i"), {}) t = use_memo( lambda: tt.update( [ @@ -398,18 +398,18 @@ def multiwave(): f"y_tan={amplitude}*Math.tan({frequency}*x+{phase})", ] ), - [amplitude, frequency, phase], + {amplitude, frequency, phase}, ) p_sin = use_memo( - lambda: Figure().plot_xy(series_name="Sine", t=t, x="x", y="y_sin").show(), [t] + lambda: Figure().plot_xy(series_name="Sine", t=t, x="x", y="y_sin").show(), {t} ) p_cos = use_memo( lambda: Figure().plot_xy(series_name="Cosine", t=t, x="x", y="y_cos").show(), - [t], + {t}, ) p_tan = use_memo( lambda: Figure().plot_xy(series_name="Tangent", t=t, x="x", y="y_tan").show(), - [t], + {t}, ) return ui.column( diff --git a/plugins/ui/src/deephaven/ui/hooks/use_effect.py b/plugins/ui/src/deephaven/ui/hooks/use_effect.py index 335432810..323ffdf9e 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_effect.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_effect.py @@ -1,9 +1,10 @@ +from typing import Any, Callable from .use_ref import use_ref from deephaven.liveness_scope import LivenessScope from .._internal import get_context -def use_effect(func, dependencies): +def use_effect(func: Callable[[], Any], dependencies: set[Any]): """ Call a function when the dependencies change. Optionally return a cleanup function to be called when dependencies change again or component is unmounted. diff --git a/plugins/ui/src/deephaven/ui/hooks/use_execution_context.py b/plugins/ui/src/deephaven/ui/hooks/use_execution_context.py index deec01f8e..90e7e1b24 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_execution_context.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_execution_context.py @@ -35,6 +35,6 @@ def use_execution_context( Returns: A callable that will take any callable and invoke it within the current exec context """ - exec_ctx = use_memo(lambda: exec_ctx if exec_ctx else get_exec_ctx(), [exec_ctx]) - exec_fn = use_memo(lambda: partial(func_with_ctx, exec_ctx), [exec_ctx]) + exec_ctx = use_memo(lambda: exec_ctx if exec_ctx else get_exec_ctx(), {exec_ctx}) + exec_fn = use_memo(lambda: partial(func_with_ctx, exec_ctx), {exec_ctx}) return exec_fn diff --git a/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py b/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py index 13bd79715..1a4ab0f21 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py @@ -1,7 +1,7 @@ from __future__ import annotations from functools import partial -from typing import Callable +from typing import Callable, Any from deephaven.table import Table from deephaven.table_listener import listen, TableUpdate, TableListener @@ -67,6 +67,7 @@ def wrap_listener( def use_table_listener( table: Table, listener: Callable[[TableUpdate, bool], None] | TableListener, + dependencies: set[Any], description: str | None = None, do_replay: bool = False, replay_lock: LockType = "shared", @@ -78,6 +79,7 @@ def use_table_listener( table: The table to listen to. listener: Either a function or a TableListener with an on_update function. The function must take a TableUpdate and is_replay bool. + dependencies: Dependencies of the listener function, so the hook knows when to recreate the listener description: An optional description for the UpdatePerformanceTracker to append to the listener’s entry description, default is None. do_replay: Whether to replay the initial snapshot of the table, default is False. @@ -105,4 +107,6 @@ def start_listener() -> Callable[[], None]: return lambda: handle.stop() - use_effect(start_listener, [table, listener, description, do_replay, replay_lock]) + use_effect( + start_listener, {table, description, do_replay, replay_lock} | dependencies + ) diff --git a/plugins/ui/test/deephaven/ui/test_hooks.py b/plugins/ui/test/deephaven/ui/test_hooks.py index 1277e2e70..bb6ad3846 100644 --- a/plugins/ui/test/deephaven/ui/test_hooks.py +++ b/plugins/ui/test/deephaven/ui/test_hooks.py @@ -106,7 +106,7 @@ def test_memo(self): from deephaven.ui.hooks import use_memo def _test_memo(fn=lambda: "foo", a=1, b=2): - return use_memo(fn, [a, b]) + return use_memo(fn, {a, b}) # Initial render render_result = render_hook(_test_memo) @@ -512,7 +512,7 @@ def start_thread(): thread.start() thread.join() - use_memo(start_thread, []) + use_memo(start_thread, set()) render_hook(_test_execution_context) @@ -535,7 +535,7 @@ def start_thread(): thread = threading.Thread(target=thread_func) thread.start() - use_memo(start_thread, []) + use_memo(start_thread, set()) render_hook(_test_execution_context) From 46b0585899e970e795de047ca6ac1ea16d34c17b Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 16 Feb 2024 13:55:47 -0600 Subject: [PATCH 2/5] Update more docs, fix up liveness scope hook --- plugins/ui/DESIGN.md | 1 + plugins/ui/examples/README.md | 4 +- .../deephaven/ui/hooks/use_liveness_scope.py | 44 +++++++++++-------- .../src/deephaven/ui/hooks/use_table_data.py | 4 +- plugins/ui/test/deephaven/ui/test_hooks.py | 4 +- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/plugins/ui/DESIGN.md b/plugins/ui/DESIGN.md index 1ea9299d6..00a72d627 100644 --- a/plugins/ui/DESIGN.md +++ b/plugins/ui/DESIGN.md @@ -1441,6 +1441,7 @@ Call a callback function or `on_update` in a `TableListener` when the table is u use_table_listener( table: Table, listener: Callable[[TableUpdate, bool], None] | TableListener, + dependencies: set[Any], description: str | None = None, do_replay: bool = False, replay_lock: LockType = "shared", diff --git a/plugins/ui/examples/README.md b/plugins/ui/examples/README.md index e161d9b0b..bb85fd36a 100644 --- a/plugins/ui/examples/README.md +++ b/plugins/ui/examples/README.md @@ -900,7 +900,7 @@ def monitor_changed_data(source: Table): else: set_changed(empty_table(0)) - ui.use_table_listener(source, listener) + ui.use_table_listener(source, listener, set()) added_check = ui.checkbox( "Show Added", isSelected=show_added, on_change=set_show_added @@ -930,7 +930,7 @@ from deephaven import ui, time_table @ui.component def resetable_table(): table, set_table = ui.use_state(lambda: time_table("PT1s")) - handle_press = ui.use_liveness_scope(lambda _: set_table(time_table("PT1s"))) + handle_press = ui.use_liveness_scope(lambda _: set_table(time_table("PT1s")), set()) return [ ui.action_button( "Reset", diff --git a/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py b/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py index 8cc404684..a386508f5 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py @@ -1,10 +1,10 @@ from .._internal import get_context -from .use_ref import use_ref -from typing import Callable +from . import use_ref, use_memo +from typing import Any, Callable from deephaven.liveness_scope import LivenessScope -def use_liveness_scope(func: Callable) -> Callable: +def use_liveness_scope(func: Callable, dependencies: set[Any]) -> Callable: """ Wraps a Callable in a liveness scope, and ensures that when invoked, if that callable causes the component to render, the scope will be retained by that render pass. It is @@ -13,25 +13,31 @@ def use_liveness_scope(func: Callable) -> Callable: Args: func: The function to wrap + dependencies: Dependencies of the provided function, so the hook knows when to re-wrap it Returns: The wrapped Callable """ + scope_ref = use_ref(None) - # If the value is set, the wrapped callable was invoked since we were last called - the current render - # cycle now will manage it, and release it when appropriate. - if scope_ref.current: - get_context().manage(scope_ref.current) - scope_ref.current = None - - def wrapped(*args, **kwargs): - # If one does not exist, create a liveness scope, to hold actions from running the provided callable. - # Instead of releasing it right away, we leave it open until the render queue picks up any changes in - # that callable, see above. - if scope_ref.current is None: - scope_ref.current = LivenessScope() - with scope_ref.current.open(): - func(*args, **kwargs) - - return wrapped + def make_wrapper(): + + # If the value is set, the wrapped callable was invoked since we were last called - the current render + # cycle now will manage it, and release it when appropriate. + if scope_ref.current: + get_context().manage(scope_ref.current) + scope_ref.current = None + + def wrapped(*args, **kwargs): + # If one does not exist, create a liveness scope, to hold actions from running the provided callable. + # Instead of releasing it right away, we leave it open until the render queue picks up any changes in + # that callable, see above. + if scope_ref.current is None: + scope_ref.current = LivenessScope() + with scope_ref.current.open(): + func(*args, **kwargs) + + return wrapped + + return use_memo(make_wrapper, dependencies) diff --git a/plugins/ui/src/deephaven/ui/hooks/use_table_data.py b/plugins/ui/src/deephaven/ui/hooks/use_table_data.py index 5041126e3..86c98d9d1 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_table_data.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_table_data.py @@ -140,6 +140,8 @@ def use_table_data( table_updated = lambda: _set_new_data(table, sentinel, set_data, set_is_sentinel) - ui.use_table_listener(table, partial(_on_update, ctx, table_updated, executor_name)) + ui.use_table_listener( + table, partial(_on_update, ctx, table_updated, executor_name), set() + ) return transform(data, is_sentinel) diff --git a/plugins/ui/test/deephaven/ui/test_hooks.py b/plugins/ui/test/deephaven/ui/test_hooks.py index bb6ad3846..6c33385d3 100644 --- a/plugins/ui/test/deephaven/ui/test_hooks.py +++ b/plugins/ui/test/deephaven/ui/test_hooks.py @@ -149,7 +149,7 @@ def listener(update: TableUpdate, is_replay: bool) -> None: event.set() def _test_table_listener(replayed_table_val=table, listener_val=listener): - use_table_listener(replayed_table_val, listener_val) + use_table_listener(replayed_table_val, listener_val, set()) render_hook(_test_table_listener) @@ -566,7 +566,7 @@ def _test_reused_tables(): a, set_a = use_state(lambda: table.where("X=1")) # When "a" changes, recompute table - don't return or otherwise track this table w.r.t. liveness - replace_a = use_liveness_scope(lambda: set_a(table.where("X=2"))) + replace_a = use_liveness_scope(lambda: set_a(table.where("X=2")), set()) return a.size From 41f4e6608d7b40598b0b7874dd8f86cf8b2956c7 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 16 Feb 2024 14:05:43 -0600 Subject: [PATCH 3/5] Make py 3.8 less grumpy? --- plugins/ui/src/deephaven/ui/hooks/use_effect.py | 2 +- plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py | 4 ++-- plugins/ui/src/deephaven/ui/hooks/use_memo.py | 2 +- plugins/ui/src/deephaven/ui/hooks/use_table_listener.py | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/hooks/use_effect.py b/plugins/ui/src/deephaven/ui/hooks/use_effect.py index 323ffdf9e..b67cceaee 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_effect.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_effect.py @@ -4,7 +4,7 @@ from .._internal import get_context -def use_effect(func: Callable[[], Any], dependencies: set[Any]): +def use_effect(func: Callable[[], Any], dependencies: set): """ Call a function when the dependencies change. Optionally return a cleanup function to be called when dependencies change again or component is unmounted. diff --git a/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py b/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py index a386508f5..96d874642 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_liveness_scope.py @@ -1,10 +1,10 @@ from .._internal import get_context from . import use_ref, use_memo -from typing import Any, Callable +from typing import Callable from deephaven.liveness_scope import LivenessScope -def use_liveness_scope(func: Callable, dependencies: set[Any]) -> Callable: +def use_liveness_scope(func: Callable, dependencies: set) -> Callable: """ Wraps a Callable in a liveness scope, and ensures that when invoked, if that callable causes the component to render, the scope will be retained by that render pass. It is diff --git a/plugins/ui/src/deephaven/ui/hooks/use_memo.py b/plugins/ui/src/deephaven/ui/hooks/use_memo.py index 1c7ec74aa..d5eb8ff0c 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_memo.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_memo.py @@ -8,7 +8,7 @@ T = TypeVar("T") -def use_memo(func: Callable[[], T], dependencies: set[Any]) -> T: +def use_memo(func: Callable[[], T], dependencies: set) -> T: """ Memoize the result of a function call. The function will only be called again if the dependencies change. diff --git a/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py b/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py index 1a4ab0f21..0d76d21d7 100644 --- a/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py +++ b/plugins/ui/src/deephaven/ui/hooks/use_table_listener.py @@ -1,7 +1,7 @@ from __future__ import annotations from functools import partial -from typing import Callable, Any +from typing import Callable from deephaven.table import Table from deephaven.table_listener import listen, TableUpdate, TableListener @@ -67,7 +67,7 @@ def wrap_listener( def use_table_listener( table: Table, listener: Callable[[TableUpdate, bool], None] | TableListener, - dependencies: set[Any], + dependencies: set, description: str | None = None, do_replay: bool = False, replay_lock: LockType = "shared", From 3cef448e8ebcd69a76dc5c2bf03ec2f7c76f963d Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 29 Feb 2024 13:38:59 -0500 Subject: [PATCH 4/5] Fix docs to use correct empty array instead of an empty dict - {} is an empty dict, not an empty set --- docker/data/storage/notebooks/DEMO.md | 4 ++-- plugins/ui/examples/README.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docker/data/storage/notebooks/DEMO.md b/docker/data/storage/notebooks/DEMO.md index 008631a72..a5a224d6f 100644 --- a/docker/data/storage/notebooks/DEMO.md +++ b/docker/data/storage/notebooks/DEMO.md @@ -230,7 +230,7 @@ def order_table(): lambda: table_publisher( "Order table", {"sym": dht.string, "size": dht.int32, "side": dht.string} ), - {}, + [], ) t = ui.use_memo(lambda: blink_to_append_only(blink_table), {blink_table}) @@ -389,7 +389,7 @@ def use_wave_input(): def multiwave(): amplitude, frequency, phase, wave_input = use_wave_input() - tt = use_memo(lambda: time_table("PT1s").update("x=i"), {}) + tt = use_memo(lambda: time_table("PT1s").update("x=i"), []) t = use_memo( lambda: tt.update( [ diff --git a/plugins/ui/examples/README.md b/plugins/ui/examples/README.md index 5066f385e..9a69c1577 100644 --- a/plugins/ui/examples/README.md +++ b/plugins/ui/examples/README.md @@ -899,7 +899,7 @@ def monitor_changed_data(source: Table): else: set_changed(empty_table(0)) - ui.use_table_listener(source, listener, set()) + ui.use_table_listener(source, listener, []) added_check = ui.checkbox( "Show Added", isSelected=show_added, on_change=set_show_added @@ -930,7 +930,7 @@ from deephaven import ui, time_table @ui.component def resetable_table(): table, set_table = ui.use_state(lambda: time_table("PT1s")) - handle_press = ui.use_liveness_scope(lambda _: set_table(time_table("PT1s")), set()) + handle_press = ui.use_liveness_scope(lambda _: set_table(time_table("PT1s")), []) return [ ui.action_button( "Reset", From 4c0bc7b17618f88b06465ad8d837520d97e48987 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 29 Feb 2024 15:30:00 -0500 Subject: [PATCH 5/5] Fix failing unit test - Also added crash logs to gitignore --- .gitignore | 4 ++++ plugins/ui/test/deephaven/ui/test_hooks.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index f431fa514..e3c012457 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,7 @@ docker-compose.override.yml # Ignore temporary files created during a release releases/ + +# virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml +hs_err_pid* +replay_pid* \ No newline at end of file diff --git a/plugins/ui/test/deephaven/ui/test_hooks.py b/plugins/ui/test/deephaven/ui/test_hooks.py index 4ee6468bd..1a9fcfb8c 100644 --- a/plugins/ui/test/deephaven/ui/test_hooks.py +++ b/plugins/ui/test/deephaven/ui/test_hooks.py @@ -151,7 +151,7 @@ def listener(update: TableUpdate, is_replay: bool) -> None: event.set() def _test_table_listener(replayed_table_val=table, listener_val=listener): - use_table_listener(replayed_table_val, listener_val, set()) + use_table_listener(replayed_table_val, listener_val, []) render_hook(_test_table_listener) @@ -171,7 +171,7 @@ def listener(update: TableUpdate, is_replay: bool) -> None: event.set() def _test_table_listener(replay_table=table, listener_val=listener): - use_table_listener(replay_table, listener_val, do_replay=True) + use_table_listener(replay_table, listener_val, [], do_replay=True) render_hook(_test_table_listener)