diff --git a/holoviews/core/options.py b/holoviews/core/options.py index 4639792e78..7df0ca20f7 100644 --- a/holoviews/core/options.py +++ b/holoviews/core/options.py @@ -37,6 +37,7 @@ import difflib import inspect import pickle +import threading import traceback import typing as t from collections import defaultdict @@ -1213,6 +1214,10 @@ class Store: _weakrefs = {} _options_context = False + # Allocator for custom-option ids; see StoreOptions.reserve_ids. + _id_counter = 0 + _id_lock = threading.Lock() + # Backend option caches _lookup_cache = {} @@ -1735,8 +1740,8 @@ def create_custom_trees(cls, obj, options, backend=None): """ clones, id_mapping = {}, [] obj_ids = cls.get_object_ids(obj) - offset = cls.id_offset() obj_ids = [None] if len(obj_ids) == 0 else obj_ids + offset = cls.reserve_ids(len(obj_ids)) used_obj_types = [(opt.split(".")[0],) for opt in options] backend = backend or Store.current_backend @@ -1754,18 +1759,16 @@ def create_custom_trees(cls, obj, options, backend=None): } custom_options = Store.custom_options(backend=backend) - for tree_id in obj_ids: + for new_id, tree_id in enumerate(obj_ids, start=offset): if tree_id is not None and tree_id in custom_options: original = custom_options[tree_id] clone = OptionTree( items=original.items(), groups=original.groups, backend=original.backend ) - clones[tree_id + offset + 1] = clone - id_mapping.append((tree_id, tree_id + offset + 1)) else: clone = OptionTree(groups=available_options.groups, backend=backend) - clones[offset] = clone - id_mapping.append((tree_id, offset)) + clones[new_id] = clone + id_mapping.append((tree_id, new_id)) # Nodes needed to ensure allowed_keywords is respected for obj_type, opts in used_options.items(): @@ -1871,6 +1874,24 @@ def id_offset(cls): # If no backends defined (e.g. plotting not imported) return zero return max(max_ids) if max_ids else 0 + @classmethod + def reserve_ids(cls, n): + """Atomically reserve a contiguous block of ``n`` fresh custom-option + ids and return its first id. + + Ids are drawn from a monotonic counter that never decreases, so a + block is never reused even as store entries are reclaimed, and + concurrent customizations receive disjoint blocks rather than + colliding on a recomputed offset. The counter is floored by + ``id_offset`` so it also clears any ids already present in the store + (e.g. restored by unpickling). + + """ + with Store._id_lock: + base = max(Store._id_counter, cls.id_offset()) + Store._id_counter = base + n + return base + @classmethod def update_backends(cls, id_mapping, custom_trees, backend=None): """Given the id_mapping from previous ids to new ids and the new diff --git a/holoviews/tests/core/test_storeoptions.py b/holoviews/tests/core/test_storeoptions.py index f55155bcf4..dfce4bf83d 100644 --- a/holoviews/tests/core/test_storeoptions.py +++ b/holoviews/tests/core/test_storeoptions.py @@ -5,6 +5,12 @@ from __future__ import annotations +import itertools +import sys +import threading +from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager + import numpy as np import holoviews as hv @@ -16,12 +22,30 @@ import holoviews.plotting.mpl +@contextmanager +def fast_switch(): + """ + Sets sys.getswitchinterval to 1 microsecond to force thread interleaving in + tests that check for id collisions with multiple threads + """ + old = sys.getswitchinterval() + sys.setswitchinterval(1e-6) + try: + yield + finally: + sys.setswitchinterval(old) + + @mpl_skip class TestStoreOptionsMerge: def setup_method(self): + self._backend = hv.Store.current_backend hv.Store.current_backend = "matplotlib" self.expected = {"Image": {"plot": {"fig_size": 150}, "style": {"cmap": "Blues"}}} + def teardown_method(self): + hv.Store.current_backend = self._backend + def test_full_spec_format(self): out = hv.StoreOptions.merge_options( ["plot", "style"], @@ -53,7 +77,11 @@ class TestStoreOptsMethod: """ def setup_method(self): - hv.Store.current_backend = "matplotlib" + self._backend = hv.Store.current_backend + hv.Store.set_current_backend("matplotlib") + + def teardown_method(self): + hv.Store.current_backend = self._backend def test_overlay_options_partitioned(self): """ @@ -111,3 +139,91 @@ def test_holomap_options(self): def test_holomap_options_empty_no_exception(self): hv.HoloMap({0: hv.Image(np.random.rand(10, 10))}).options() + + +@mpl_skip +class TestStoreOptionsCustomIdGrowth: + """ + Re-customizing retained, already-customized objects must keep custom-option + ids bounded and must not let concurrent customizations collide on an id. + """ + + def setup_method(self): + self._backend = hv.Store.current_backend + hv.Store.set_current_backend("matplotlib") + + def teardown_method(self): + hv.Store.current_backend = self._backend + + @staticmethod + def _max_custom_id(): + keys = list(hv.Store._custom_options.get("matplotlib", {}).keys()) + return max(keys) if keys else 0 + + def test_recustomization_id_growth_is_not_exponential(self): + el = hv.Curve([1, 2, 3]) + max_ids = [] + for _ in range(20): + el = el.opts(color="red") + max_ids.append(self._max_custom_id()) + + growth = [b - a for a, b in itertools.pairwise(max_ids)] + assert max(growth) <= 4, f"id growth per call is not bounded: {max_ids}" + + def test_cross_backend_recustomization_keeps_children_distinct(self): + c1 = hv.Curve([1, 2, 3]).opts(color="red", backend="bokeh") + c2 = hv.Curve([3, 2, 1]).opts(color="green", backend="bokeh") + ov = (c1 * c2).opts({"Curve": {"linewidth": 3}}, backend="matplotlib") + + first, second = ov.Curve.I, ov.Curve.II + assert first.id != second.id + assert hv.Store.lookup_options("bokeh", first, "style").kwargs["color"] == "red" + assert hv.Store.lookup_options("bokeh", second, "style").kwargs["color"] == "green" + + def test_overlay_subset_update_keeps_ids_bounded(self): + names = list("abc") + curves = {k: hv.Curve([(0, 0)], label=k) for k in names} + base = self._max_custom_id() + max_ids = [] + for cycle in range(15): + k = names[cycle % len(names)] # only one curve gets fresh data + curves[k] = hv.Curve([(x, x + cycle) for x in range(cycle + 2)], label=k) + hv.Overlay(list(curves.values())).opts({"Curve": {"linewidth": 2}}) + max_ids.append(self._max_custom_id() - base) + + assert max_ids[-1] < 500, f"id growth is not polynomial: {max_ids}" + + def test_concurrent_recustomization_keeps_styles_distinct(self): + n_workers = 4 + barrier = threading.Barrier(n_workers) + + def worker(w): + barrier.wait() + mismatches = 0 + for _ in range(60): + el = hv.Curve([1, 2, 3], label=f"w{w}").opts(linewidth=w + 1) + got = hv.Store.lookup_options("matplotlib", el, "style").kwargs.get("linewidth") + mismatches += got != w + 1 + return mismatches + + with fast_switch(), ThreadPoolExecutor(max_workers=n_workers) as executor: + mismatches = sum(executor.map(worker, range(n_workers))) + + assert mismatches == 0, f"concurrent customizations collided on ids: {mismatches}" + + def test_concurrent_id_reservation_yields_disjoint_blocks(self): + n_workers = 4 + barrier = threading.Barrier(n_workers) + + def worker(_): + barrier.wait() + ids = [] + for n in range(10, 30): + start = hv.StoreOptions.reserve_ids(n) + ids.extend(range(start, start + n)) + return ids + + with fast_switch(), ThreadPoolExecutor(max_workers=n_workers) as executor: + reserved = [i for block in executor.map(worker, range(n_workers)) for i in block] + + assert len(reserved) == len(set(reserved)), "reserved id blocks overlap"