From c0af0897c8f6f7bf542d78c18d609963f0b9971b Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 19:52:16 -0400 Subject: [PATCH 01/18] Add optional Rust native helper scaffold Make the Rust toolchain hermetic via nix and wire an optional PyO3 helper package into the development workflow. Add an initial native fast path for building dict-backed bidict state, along with tests and related ignore/doc updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .gitignore | 6 + CONTRIBUTING.rst | 15 +- bidict/_base.py | 5 + bidict/_native.py | 41 +++++ flake.nix | 5 +- init_dev_env | 2 +- pyproject.toml | 8 +- rust/bidict_base_opt_native/Cargo.lock | 180 +++++++++++++++++++++ rust/bidict_base_opt_native/Cargo.toml | 13 ++ rust/bidict_base_opt_native/pyproject.toml | 14 ++ rust/bidict_base_opt_native/src/lib.rs | 119 ++++++++++++++ tests/test_native.py | 89 ++++++++++ uv.lock | 9 ++ 13 files changed, 497 insertions(+), 9 deletions(-) create mode 100644 bidict/_native.py create mode 100644 rust/bidict_base_opt_native/Cargo.lock create mode 100644 rust/bidict_base_opt_native/Cargo.toml create mode 100644 rust/bidict_base_opt_native/pyproject.toml create mode 100644 rust/bidict_base_opt_native/src/lib.rs create mode 100644 tests/test_native.py diff --git a/.gitignore b/.gitignore index b1190e8e..e278599f 100644 --- a/.gitignore +++ b/.gitignore @@ -10,9 +10,14 @@ __pycache__ .hypothesis .idea .mypy_cache +.pytest_cache +.ruff_cache .tox .venv +.venv-* .venv-benchmark +.jump +benchmark_linux_arm64.sh bidict.egg-info build _build @@ -20,3 +25,4 @@ coverage.xml dist htmlcov pip-wheel-metadata +target diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index f6efe08c..64467fff 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -44,14 +44,17 @@ Making Changes - If you have `Nix `__, run ``nix develop`` from within your clone to start a shell where all supported - Python versions as well as ``prek`` are installed and added - to your PATH. This also pins ``uv`` to the flake-provided - default Python when initializing ``.venv``. + Python versions as well as ``prek``, ``uv``, ``rustc``, + ``cargo``, and ``maturin`` are installed and added to your PATH. + This also pins ``uv`` to the flake-provided default Python + when initializing ``.venv``. - Otherwise, manually ensure you have `uv `__, - `prek `__, and at least the latest - `stable Python version `__ installed - and on your PATH. + `prek `__, + `Rust `__ (including + ``cargo`` and ``rustc``), `maturin `__, + and at least the latest `stable Python version `__ + installed and on your PATH. - Run ``./init_dev_env`` diff --git a/bidict/_base.py b/bidict/_base.py index 2de3cc58..58ea98f3 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -41,6 +41,7 @@ from ._exc import ValueDuplicationError from ._iter import inverted from ._iter import iteritems +from ._native import build_bidict_maps as _build_bidict_maps from ._typing import KT from ._typing import MISSING from ._typing import OKT @@ -449,6 +450,10 @@ def _update( self._init_from(arg) return + if not self and self._fwdm_cls is dict and self._invm_cls is dict and _build_bidict_maps is not None: + self._fwdm, self._invm = _build_bidict_maps(iteritems(arg, **kw), on_dup) + return + # Fast path when we're adding more items than we contain already and rollback is enabled: # Update a copy of self with rollback disabled. Fail if that fails, otherwise become the copy. if rollback and isinstance(arg, t.Sized) and len(arg) + len(kw) > len(self): diff --git a/bidict/_native.py b/bidict/_native.py new file mode 100644 index 00000000..058968d5 --- /dev/null +++ b/bidict/_native.py @@ -0,0 +1,41 @@ +# Copyright 2009-2026 Joshua Bronson. All rights reserved. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +"""Optional native helpers for accelerating selected bidict operations.""" + +from __future__ import annotations + +import typing as t +from collections.abc import Iterable + +from ._dup import OnDup + + +Items = Iterable[tuple[t.Any, t.Any]] +BuildBidictMaps: t.TypeAlias = t.Callable[[Items, OnDup], tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]] +build_bidict_maps: BuildBidictMaps | None + + +if t.TYPE_CHECKING: + build_bidict_maps = None + + def _build_bidict_maps_impl( + items: Items, + on_dup_key: str, + on_dup_val: str, + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... + +else: + try: + from bidict_base_opt_native import build_bidict_maps as _build_bidict_maps_impl + except ModuleNotFoundError as exc: + if exc.name != 'bidict_base_opt_native': + raise + build_bidict_maps: BuildBidictMaps | None = None + else: + + def build_bidict_maps(items: Items, on_dup: OnDup) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: + return _build_bidict_maps_impl(items, on_dup.key.name, on_dup.val.name) diff --git a/flake.nix b/flake.nix index 0d4ddcd3..4c63fbeb 100644 --- a/flake.nix +++ b/flake.nix @@ -13,6 +13,7 @@ lib = pkgs.lib; latestPython = pkgs.python314; commonTools = with pkgs; [prek uv]; + nativeTools = with pkgs; [cargo rustc maturin]; supportedPythons = with pkgs; [ python314 python313 @@ -74,7 +75,7 @@ devShells = { default = let - packages = commonTools ++ supportedPythons; + packages = commonTools ++ nativeTools ++ supportedPythons; in pkgs.mkShell { inherit packages; @@ -96,6 +97,7 @@ }; build = mkUvShell { python = pkgs.python313; + extraPackages = nativeTools; }; lint = pkgs.mkShell { packages = with pkgs; [prek]; @@ -123,6 +125,7 @@ }; update_deps = mkUvShell { python = latestPython; + extraPackages = nativeTools; }; }; }); diff --git a/init_dev_env b/init_dev_env index ea31df74..536c6616 100755 --- a/init_dev_env +++ b/init_dev_env @@ -12,7 +12,7 @@ set -euo pipefail declare -r hint="Hint: Use 'nix develop' to bootstrap a development environment" -for cmd in uv prek; do +for cmd in uv prek rustc cargo maturin; do if ! command -v "$cmd" >/dev/null 2>&1; then >&2 echo "Error: No '$cmd' on PATH. $hint" exit 1 diff --git a/pyproject.toml b/pyproject.toml index de267948..fcdd5969 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,9 +60,15 @@ docs = [ "sphinx-copybutton", "furo", ] +native = [ + "bidict-base-opt-native", +] [tool.uv] -default-groups = ["test", "dev", "docs"] +default-groups = ["test", "dev", "docs", "native"] + +[tool.uv.sources] +bidict-base-opt-native = { path = "rust/bidict_base_opt_native", editable = true } [tool.uv.build-backend] module-root = "" diff --git a/rust/bidict_base_opt_native/Cargo.lock b/rust/bidict_base_opt_native/Cargo.lock new file mode 100644 index 00000000..3d5cdcc2 --- /dev/null +++ b/rust/bidict_base_opt_native/Cargo.lock @@ -0,0 +1,180 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + +[[package]] +name = "bidict-base-opt-native" +version = "0.23.2-dev0" +dependencies = [ + "pyo3", +] + +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + +[[package]] +name = "heck" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" + +[[package]] +name = "indoc" +version = "2.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79cf5c93f93228cf8efb3ba362535fb11199ac548a09ce117c9b1adc3030d706" +dependencies = [ + "rustversion", +] + +[[package]] +name = "libc" +version = "0.2.186" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" + +[[package]] +name = "memoffset" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "488016bfae457b036d996092f6cb448677611ce4449e970ceaf42695203f218a" +dependencies = [ + "autocfg", +] + +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + +[[package]] +name = "portable-atomic" +version = "1.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c33a9471896f1c69cecef8d20cbe2f7accd12527ce60845ff44c153bb2a21b49" + +[[package]] +name = "proc-macro2" +version = "1.0.106" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "pyo3" +version = "0.22.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f402062616ab18202ae8319da13fa4279883a2b8a9d9f83f20dbade813ce1884" +dependencies = [ + "cfg-if", + "indoc", + "libc", + "memoffset", + "once_cell", + "portable-atomic", + "pyo3-build-config", + "pyo3-ffi", + "pyo3-macros", + "unindent", +] + +[[package]] +name = "pyo3-build-config" +version = "0.22.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b14b5775b5ff446dd1056212d778012cbe8a0fbffd368029fd9e25b514479c38" +dependencies = [ + "once_cell", + "target-lexicon", +] + +[[package]] +name = "pyo3-ffi" +version = "0.22.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ab5bcf04a2cdcbb50c7d6105de943f543f9ed92af55818fd17b660390fc8636" +dependencies = [ + "libc", + "pyo3-build-config", +] + +[[package]] +name = "pyo3-macros" +version = "0.22.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fd24d897903a9e6d80b968368a34e1525aeb719d568dba8b3d4bfa5dc67d453" +dependencies = [ + "proc-macro2", + "pyo3-macros-backend", + "quote", + "syn", +] + +[[package]] +name = "pyo3-macros-backend" +version = "0.22.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "36c011a03ba1e50152b4b394b479826cad97e7a21eb52df179cd91ac411cbfbe" +dependencies = [ + "heck", + "proc-macro2", + "pyo3-build-config", + "quote", + "syn", +] + +[[package]] +name = "quote" +version = "1.0.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rustversion" +version = "1.0.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" + +[[package]] +name = "syn" +version = "2.0.117" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "target-lexicon" +version = "0.12.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61c41af27dd6d1e27b1b16b489db798443478cef1f06a660c96db617ba5de3b1" + +[[package]] +name = "unicode-ident" +version = "1.0.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" + +[[package]] +name = "unindent" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7264e107f553ccae879d21fbea1d6724ac785e8c3bfc762137959b5802826ef3" diff --git a/rust/bidict_base_opt_native/Cargo.toml b/rust/bidict_base_opt_native/Cargo.toml new file mode 100644 index 00000000..db098e6c --- /dev/null +++ b/rust/bidict_base_opt_native/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "bidict-base-opt-native" +version = "0.23.2-dev0" +edition = "2021" +license = "MPL-2.0" +publish = false + +[lib] +name = "bidict_base_opt_native" +crate-type = ["cdylib"] + +[dependencies] +pyo3 = { version = "0.22.6", features = ["abi3-py311", "extension-module"] } diff --git a/rust/bidict_base_opt_native/pyproject.toml b/rust/bidict_base_opt_native/pyproject.toml new file mode 100644 index 00000000..4158e9e0 --- /dev/null +++ b/rust/bidict_base_opt_native/pyproject.toml @@ -0,0 +1,14 @@ +[project] +name = "bidict-base-opt-native" +version = "0.23.2.dev0" +description = "Optional native helpers for bidict." +authors = [{ name = "Joshua Bronson", email = "jabronson@gmail.com" }] +license = "MPL-2.0" +requires-python = ">=3.11" + +[build-system] +requires = ["maturin>=1.8,<2.0"] +build-backend = "maturin" + +[tool.maturin] +manifest-path = "Cargo.toml" diff --git a/rust/bidict_base_opt_native/src/lib.rs b/rust/bidict_base_opt_native/src/lib.rs new file mode 100644 index 00000000..1f609389 --- /dev/null +++ b/rust/bidict_base_opt_native/src/lib.rs @@ -0,0 +1,119 @@ +use pyo3::prelude::*; +use pyo3::types::PyDict; +use pyo3::types::PyType; + + +#[derive(Clone, Copy)] +enum OnDupAction { + Raise, + DropOld, + DropNew, +} + + +impl OnDupAction { + fn parse(value: &str) -> PyResult { + match value { + "RAISE" => Ok(Self::Raise), + "DROP_OLD" => Ok(Self::DropOld), + "DROP_NEW" => Ok(Self::DropNew), + _ => Err(pyo3::exceptions::PyValueError::new_err(format!("unknown OnDupAction: {value}"))), + } + } +} + + +fn bidict_err(py: Python<'_>, name: &str, args: A) -> PyErr +where + A: pyo3::PyErrArguments + Send + Sync + 'static, +{ + let bidict = py + .import_bound("bidict") + .expect("bidict should already be importable"); + let err_type = bidict + .getattr(name) + .expect("bidict exception should already be importable") + .downcast_into::() + .expect("bidict exception should be a Python type"); + PyErr::from_type_bound(err_type, args) +} + + +#[pyfunction] +fn build_bidict_maps( + py: Python<'_>, + items: Bound<'_, PyAny>, + on_dup_key: &str, + on_dup_val: &str, +) -> PyResult<(Py, Py)> { + let on_dup_key = OnDupAction::parse(on_dup_key)?; + let on_dup_val = OnDupAction::parse(on_dup_val)?; + let fwd = PyDict::new_bound(py); + let inv = PyDict::new_bound(py); + + for item in items.iter()? { + let item = item?; + let (key, val): (Py, Py) = item.extract()?; + let oldval = fwd.get_item(key.bind(py))?.map(Bound::unbind); + let oldkey = inv.get_item(val.bind(py))?.map(Bound::unbind); + let isdupkey = oldval.is_some(); + let isdupval = oldkey.is_some(); + + if isdupkey && isdupval { + let oldkey = oldkey.as_ref().expect("checked is_some above"); + let oldval = oldval.as_ref().expect("checked is_some above"); + if key.bind(py).eq(oldkey.bind(py))? { + assert!(val.bind(py).eq(oldval.bind(py))?); + continue; + } + match on_dup_val { + OnDupAction::Raise => { + return Err(bidict_err(py, "KeyAndValueDuplicationError", (key.clone_ref(py), val.clone_ref(py)))); + } + OnDupAction::DropNew => continue, + OnDupAction::DropOld => {} + } + } else if isdupkey { + match on_dup_key { + OnDupAction::Raise => { + return Err(bidict_err(py, "KeyDuplicationError", (key.clone_ref(py),))); + } + OnDupAction::DropNew => continue, + OnDupAction::DropOld => {} + } + } else if isdupval { + match on_dup_val { + OnDupAction::Raise => { + return Err(bidict_err(py, "ValueDuplicationError", (val.clone_ref(py),))); + } + OnDupAction::DropNew => continue, + OnDupAction::DropOld => {} + } + } + + fwd.set_item(key.bind(py), val.bind(py))?; + inv.set_item(val.bind(py), key.bind(py))?; + + if isdupkey && isdupval { + let oldkey = oldkey.as_ref().expect("checked is_some above"); + let oldval = oldval.as_ref().expect("checked is_some above"); + fwd.del_item(oldkey.bind(py))?; + inv.del_item(oldval.bind(py))?; + } else if isdupkey { + let oldval = oldval.as_ref().expect("checked is_some above"); + inv.del_item(oldval.bind(py))?; + } else if isdupval { + let oldkey = oldkey.as_ref().expect("checked is_some above"); + fwd.del_item(oldkey.bind(py))?; + } + } + + Ok((fwd.unbind(), inv.unbind())) +} + + +#[pymodule] +fn bidict_base_opt_native(_py: Python<'_>, module: &Bound<'_, PyModule>) -> PyResult<()> { + module.add_function(wrap_pyfunction!(build_bidict_maps, module)?)?; + Ok(()) +} diff --git a/tests/test_native.py b/tests/test_native.py new file mode 100644 index 00000000..3a6e3bd7 --- /dev/null +++ b/tests/test_native.py @@ -0,0 +1,89 @@ +# Copyright 2009-2026 Joshua Bronson. All rights reserved. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +from __future__ import annotations + +from collections.abc import Iterable + +import pytest + +from bidict import DROP_NEW +from bidict import ON_DUP_DROP_OLD +from bidict import KeyAndValueDuplicationError +from bidict import OnDup +from bidict import ValueDuplicationError +from bidict import _base as base_mod +from bidict import _native as native_mod +from bidict import bidict + + +def test_empty_update_uses_native_builder_when_available(monkeypatch: pytest.MonkeyPatch) -> None: + items_seen: list[tuple[int, int]] = [] + + def fake_build(items: Iterable[tuple[int, int]], _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: + items_seen.extend(items) + return {1: 2}, {2: 1} + + monkeypatch.setattr(base_mod, '_build_bidict_maps', fake_build) + bi = bidict[int, int]() + + bi.update([(1, 2)]) + + assert items_seen == [(1, 2)] + assert dict(bi.items()) == {1: 2} + assert dict(bi.inverse.items()) == {2: 1} + + +def test_nonempty_update_skips_native_builder(monkeypatch: pytest.MonkeyPatch) -> None: + bi = bidict({1: 2}) + + def fail_build(_items: object, _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: + msg = 'native builder should not run for non-empty updates' + raise AssertionError(msg) + + monkeypatch.setattr(base_mod, '_build_bidict_maps', fail_build) + + bi.update([(3, 4)]) + + assert dict(bi.items()) == {1: 2, 3: 4} + + +native_build = native_mod.build_bidict_maps +if native_build is not None: + pytest.importorskip('bidict_base_opt_native') + + +@pytest.mark.skipif(native_build is None, reason='optional native helper is not installed') +def test_native_build_bidict_maps_matches_drop_old_behavior() -> None: + assert native_build is not None + fwd, inv = native_build([(1, 2), (1, 3), (4, 3)], ON_DUP_DROP_OLD) + + assert fwd == {4: 3} + assert inv == {3: 4} + + +@pytest.mark.skipif(native_build is None, reason='optional native helper is not installed') +def test_native_build_bidict_maps_honors_drop_new() -> None: + assert native_build is not None + on_dup = OnDup(key=DROP_NEW, val=DROP_NEW) + fwd, inv = native_build([(1, 2), (1, 3), (4, 2)], on_dup) + + assert fwd == {1: 2} + assert inv == {2: 1} + + +@pytest.mark.skipif(native_build is None, reason='optional native helper is not installed') +def test_native_build_bidict_maps_raises_value_duplication_error() -> None: + assert native_build is not None + with pytest.raises(ValueDuplicationError): + native_build([(1, 2), (3, 2)], bidict.on_dup) + + +@pytest.mark.skipif(native_build is None, reason='optional native helper is not installed') +def test_native_build_bidict_maps_raises_key_and_value_duplication_error() -> None: + assert native_build is not None + with pytest.raises(KeyAndValueDuplicationError): + native_build([(1, 2), (3, 4), (1, 4)], bidict.on_dup) diff --git a/uv.lock b/uv.lock index 8ce9cdfe..66872d5a 100644 --- a/uv.lock +++ b/uv.lock @@ -112,6 +112,9 @@ docs = [ { name = "sphinx", version = "9.1.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.12'" }, { name = "sphinx-copybutton" }, ] +native = [ + { name = "bidict-base-opt-native" }, +] test = [ { name = "coverage" }, { name = "hypothesis" }, @@ -141,6 +144,7 @@ docs = [ { name = "sphinx" }, { name = "sphinx-copybutton" }, ] +native = [{ name = "bidict-base-opt-native", editable = "rust/bidict_base_opt_native" }] test = [ { name = "coverage" }, { name = "hypothesis" }, @@ -154,6 +158,11 @@ test = [ { name = "typing-extensions" }, ] +[[package]] +name = "bidict-base-opt-native" +version = "0.23.2.dev0" +source = { editable = "rust/bidict_base_opt_native" } + [[package]] name = "cachetools" version = "7.1.2" From 1316aa89ce7a8db5c37167873e49beed2a8dbf25 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 19:54:19 -0400 Subject: [PATCH 02/18] Track local Linux benchmark helper Stop ignoring benchmark_linux_arm64.sh and add it to the repository as a local helper for running the existing cachegrind microbenchmark workflow on this Mac via a Linux container. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .gitignore | 1 - benchmark_linux_arm64.sh | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100755 benchmark_linux_arm64.sh diff --git a/.gitignore b/.gitignore index e278599f..7d3c3202 100644 --- a/.gitignore +++ b/.gitignore @@ -17,7 +17,6 @@ __pycache__ .venv-* .venv-benchmark .jump -benchmark_linux_arm64.sh bidict.egg-info build _build diff --git a/benchmark_linux_arm64.sh b/benchmark_linux_arm64.sh new file mode 100755 index 00000000..9ec45f60 --- /dev/null +++ b/benchmark_linux_arm64.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# +# Copyright 2009-2026 Joshua Bronson. All rights reserved. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +set -euo pipefail + +if ! command -v container >/dev/null 2>&1; then + >&2 echo "Error: No 'container' command on PATH." + exit 1 +fi + +repo_root=$( + cd -- "$(dirname -- "${BASH_SOURCE[0]}")" + pwd +) + +container run --rm --progress plain \ + -v "${repo_root}:/work" \ + -w /work \ + ubuntu:24.04 \ + bash -lc ' + set -euo pipefail + export DEBIAN_FRONTEND=noninteractive + export PYTHONHASHSEED=42 + export UV_LINK_MODE=copy + export UV_PROJECT_ENVIRONMENT=/tmp/bidict-bench-venv + + apt-get update >/dev/null + apt-get install -y ca-certificates git util-linux valgrind python3 python3-venv python3-pip >/dev/null + python3 -m pip install --break-system-packages uv >/dev/null + + uv sync --only-group test >/dev/null + . /tmp/bidict-bench-venv/bin/activate + + ./cachegrind.py python -m pytest -c /dev/null -n0 \ + --benchmark-columns=min,rounds,iterations \ + --benchmark-disable-gc \ + --benchmark-group-by=name \ + microbenchmarks.py + ' From db27ac7227e8ae3d4986bf5fe623f1fa915979d0 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 20:32:36 -0400 Subject: [PATCH 03/18] Optimize native bidict updates Tighten the Linux benchmark helper to use a pinned modern Rust image and extend the optional native helper to accelerate rollback-safe bulk updates into existing dict-backed bidicts. Add coverage for the new update path and inverse-preservation behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- benchmark_linux_arm64.sh | 24 +++++- bidict/_base.py | 31 ++++++- bidict/_native.py | 31 ++++++- rust/bidict_base_opt_native/src/lib.rs | 55 ++++++++++--- tests/test_native.py | 108 ++++++++++++++++++++++++- 5 files changed, 231 insertions(+), 18 deletions(-) diff --git a/benchmark_linux_arm64.sh b/benchmark_linux_arm64.sh index 9ec45f60..18bee4ff 100755 --- a/benchmark_linux_arm64.sh +++ b/benchmark_linux_arm64.sh @@ -8,6 +8,8 @@ set -euo pipefail +declare -r container_image='rust:1.95-bookworm' + if ! command -v container >/dev/null 2>&1; then >&2 echo "Error: No 'container' command on PATH." exit 1 @@ -21,8 +23,8 @@ repo_root=$( container run --rm --progress plain \ -v "${repo_root}:/work" \ -w /work \ - ubuntu:24.04 \ - bash -lc ' + "${container_image}" \ + bash -c ' set -euo pipefail export DEBIAN_FRONTEND=noninteractive export PYTHONHASHSEED=42 @@ -30,12 +32,26 @@ container run --rm --progress plain \ export UV_PROJECT_ENVIRONMENT=/tmp/bidict-bench-venv apt-get update >/dev/null - apt-get install -y ca-certificates git util-linux valgrind python3 python3-venv python3-pip >/dev/null + apt-get install -y \ + build-essential \ + ca-certificates \ + python3 \ + python3-dev \ + python3-pip \ + python3-venv \ + util-linux \ + valgrind \ + >/dev/null python3 -m pip install --break-system-packages uv >/dev/null - uv sync --only-group test >/dev/null + rustc --version + cargo --version + + uv sync --all-groups --frozen >/dev/null . /tmp/bidict-bench-venv/bin/activate + python -c "import bidict._native as native; assert native.build_bidict_maps is not None; print(\"native helper:\", native.build_bidict_maps.__module__)" + ./cachegrind.py python -m pytest -c /dev/null -n0 \ --benchmark-columns=min,rounds,iterations \ --benchmark-disable-gc \ diff --git a/bidict/_base.py b/bidict/_base.py index 58ea98f3..d07e70f6 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -42,6 +42,7 @@ from ._iter import inverted from ._iter import iteritems from ._native import build_bidict_maps as _build_bidict_maps +from ._native import update_bidict_maps as _update_bidict_maps from ._typing import KT from ._typing import MISSING from ._typing import OKT @@ -212,6 +213,17 @@ def _make_inverse(self) -> BidictBase[VT, KT]: inv._invm = self._fwdm return inv + def _set_map_data(self, fwdm: MutableMapping[KT, VT], invm: MutableMapping[VT, KT]) -> None: + """Replace our backing maps, preserving any already-materialized inverse instance.""" + if getattr(self, '_inv', None) is None and getattr(self, '_invweak', None) is None: + self._fwdm = fwdm + self._invm = invm + return + self._fwdm.clear() + self._invm.clear() + self._fwdm.update(fwdm) + self._invm.update(invm) + @property def inv(self) -> BidictBase[VT, KT]: """Alias for :attr:`inverse`.""" @@ -444,6 +456,7 @@ def _update( on_dup = self.on_dup if rollback is None: rollback = RAISE in on_dup + incoming_len = len(arg) + len(kw) if isinstance(arg, t.Sized) else None # Fast path when we're empty and updating only from another bidict (i.e. no dup vals in new items). if not self and not kw and isinstance(arg, BidictBase): @@ -451,12 +464,26 @@ def _update( return if not self and self._fwdm_cls is dict and self._invm_cls is dict and _build_bidict_maps is not None: - self._fwdm, self._invm = _build_bidict_maps(iteritems(arg, **kw), on_dup) + self._set_map_data(*_build_bidict_maps(iteritems(arg, **kw), on_dup)) + return + + if ( + self + and rollback + and incoming_len is not None + and incoming_len >= len(self) + and self._fwdm_cls is dict + and self._invm_cls is dict + and _update_bidict_maps is not None + ): + fwdm = t.cast(dict[t.Any, t.Any], self._fwdm) + invm = t.cast(dict[t.Any, t.Any], self._invm) + self._set_map_data(*_update_bidict_maps(fwdm, invm, iteritems(arg, **kw), on_dup)) return # Fast path when we're adding more items than we contain already and rollback is enabled: # Update a copy of self with rollback disabled. Fail if that fails, otherwise become the copy. - if rollback and isinstance(arg, t.Sized) and len(arg) + len(kw) > len(self): + if rollback and incoming_len is not None and incoming_len > len(self): tmp = self.copy() tmp._update(arg, kw, rollback=False, on_dup=on_dup) self._init_from(tmp) diff --git a/bidict/_native.py b/bidict/_native.py index 058968d5..e2d65bb8 100644 --- a/bidict/_native.py +++ b/bidict/_native.py @@ -16,11 +16,17 @@ Items = Iterable[tuple[t.Any, t.Any]] BuildBidictMaps: t.TypeAlias = t.Callable[[Items, OnDup], tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]] +UpdateBidictMaps: t.TypeAlias = t.Callable[ + [dict[t.Any, t.Any], dict[t.Any, t.Any], Items, OnDup], + tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]], +] build_bidict_maps: BuildBidictMaps | None +update_bidict_maps: UpdateBidictMaps | None if t.TYPE_CHECKING: build_bidict_maps = None + update_bidict_maps = None def _build_bidict_maps_impl( items: Items, @@ -28,14 +34,37 @@ def _build_bidict_maps_impl( on_dup_val: str, ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... + def _update_bidict_maps_impl( + fwd: dict[t.Any, t.Any], + inv: dict[t.Any, t.Any], + items: Items, + on_dup_key: str, + on_dup_val: str, + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... + else: try: - from bidict_base_opt_native import build_bidict_maps as _build_bidict_maps_impl + from bidict_base_opt_native import bidict_base_opt_native as _native_ext except ModuleNotFoundError as exc: if exc.name != 'bidict_base_opt_native': raise build_bidict_maps: BuildBidictMaps | None = None + update_bidict_maps: UpdateBidictMaps | None = None else: + _build_bidict_maps_impl = _native_ext.build_bidict_maps + _update_bidict_maps_impl = getattr(_native_ext, 'update_bidict_maps', None) def build_bidict_maps(items: Items, on_dup: OnDup) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: return _build_bidict_maps_impl(items, on_dup.key.name, on_dup.val.name) + + if _update_bidict_maps_impl is None: + update_bidict_maps = None + else: + + def update_bidict_maps( + fwd: dict[t.Any, t.Any], + inv: dict[t.Any, t.Any], + items: Items, + on_dup: OnDup, + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: + return _update_bidict_maps_impl(fwd, inv, items, on_dup.key.name, on_dup.val.name) diff --git a/rust/bidict_base_opt_native/src/lib.rs b/rust/bidict_base_opt_native/src/lib.rs index 1f609389..1c335383 100644 --- a/rust/bidict_base_opt_native/src/lib.rs +++ b/rust/bidict_base_opt_native/src/lib.rs @@ -39,18 +39,14 @@ where } -#[pyfunction] -fn build_bidict_maps( +fn apply_items( py: Python<'_>, + fwd: &Bound<'_, PyDict>, + inv: &Bound<'_, PyDict>, items: Bound<'_, PyAny>, - on_dup_key: &str, - on_dup_val: &str, -) -> PyResult<(Py, Py)> { - let on_dup_key = OnDupAction::parse(on_dup_key)?; - let on_dup_val = OnDupAction::parse(on_dup_val)?; - let fwd = PyDict::new_bound(py); - let inv = PyDict::new_bound(py); - + on_dup_key: OnDupAction, + on_dup_val: OnDupAction, +) -> PyResult<()> { for item in items.iter()? { let item = item?; let (key, val): (Py, Py) = item.extract()?; @@ -108,12 +104,51 @@ fn build_bidict_maps( } } + Ok(()) +} + + +#[pyfunction] +fn build_bidict_maps( + py: Python<'_>, + items: Bound<'_, PyAny>, + on_dup_key: &str, + on_dup_val: &str, +) -> PyResult<(Py, Py)> { + let on_dup_key = OnDupAction::parse(on_dup_key)?; + let on_dup_val = OnDupAction::parse(on_dup_val)?; + let fwd = PyDict::new_bound(py); + let inv = PyDict::new_bound(py); + + apply_items(py, &fwd, &inv, items, on_dup_key, on_dup_val)?; + Ok((fwd.unbind(), inv.unbind())) } +#[pyfunction] +fn update_bidict_maps( + py: Python<'_>, + fwd: Bound<'_, PyDict>, + inv: Bound<'_, PyDict>, + items: Bound<'_, PyAny>, + on_dup_key: &str, + on_dup_val: &str, +) -> PyResult<(Py, Py)> { + let on_dup_key = OnDupAction::parse(on_dup_key)?; + let on_dup_val = OnDupAction::parse(on_dup_val)?; + let new_fwd = fwd.copy()?; + let new_inv = inv.copy()?; + + apply_items(py, &new_fwd, &new_inv, items, on_dup_key, on_dup_val)?; + + Ok((new_fwd.unbind(), new_inv.unbind())) +} + + #[pymodule] fn bidict_base_opt_native(_py: Python<'_>, module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_function(wrap_pyfunction!(build_bidict_maps, module)?)?; + module.add_function(wrap_pyfunction!(update_bidict_maps, module)?)?; Ok(()) } diff --git a/tests/test_native.py b/tests/test_native.py index 3a6e3bd7..76d72191 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -37,6 +37,24 @@ def fake_build(items: Iterable[tuple[int, int]], _on_dup: object) -> tuple[dict[ assert dict(bi.inverse.items()) == {2: 1} +def test_empty_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: + items_seen: list[tuple[int, int]] = [] + bi = bidict[int, int]() + inv = bi.inverse + + def fake_build(items: Iterable[tuple[int, int]], _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: + items_seen.extend(items) + return {1: 2}, {2: 1} + + monkeypatch.setattr(base_mod, '_build_bidict_maps', fake_build) + + bi.update([(1, 2)]) + + assert items_seen == [(1, 2)] + assert dict(inv.items()) == {2: 1} + assert inv.inverse is bi + + def test_nonempty_update_skips_native_builder(monkeypatch: pytest.MonkeyPatch) -> None: bi = bidict({1: 2}) @@ -51,8 +69,69 @@ def fail_build(_items: object, _on_dup: object) -> tuple[dict[int, int], dict[in assert dict(bi.items()) == {1: 2, 3: 4} +def test_nonempty_bulk_update_uses_native_updater_when_available(monkeypatch: pytest.MonkeyPatch) -> None: + items_seen: list[tuple[int, int]] = [] + + def fake_update( + fwd: dict[int, int], + inv: dict[int, int], + items: Iterable[tuple[int, int]], + _on_dup: object, + ) -> tuple[dict[int, int], dict[int, int]]: + items_seen.extend(items) + assert fwd == {1: 2, 3: 4} + assert inv == {2: 1, 4: 3} + return {1: 2, 3: 4, 5: 6, 7: 8}, {2: 1, 4: 3, 6: 5, 8: 7} + + monkeypatch.setattr(base_mod, '_update_bidict_maps', fake_update) + bi = bidict({1: 2, 3: 4}) + + bi.update([(5, 6), (7, 8)]) + + assert items_seen == [(5, 6), (7, 8)] + assert dict(bi.items()) == {1: 2, 3: 4, 5: 6, 7: 8} + + +def test_nonempty_small_update_skips_native_updater(monkeypatch: pytest.MonkeyPatch) -> None: + bi = bidict({1: 2, 3: 4}) + + def fail_update( + _fwd: object, _inv: object, _items: object, _on_dup: object + ) -> tuple[dict[int, int], dict[int, int]]: + msg = 'native updater should not run for small updates' + raise AssertionError(msg) + + monkeypatch.setattr(base_mod, '_update_bidict_maps', fail_update) + + bi.update([(5, 6)]) + + assert dict(bi.items()) == {1: 2, 3: 4, 5: 6} + + +def test_nonempty_native_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: + bi = bidict({1: 2, 3: 4}) + inv = bi.inverse + + def fake_update( + _fwd: dict[int, int], + _inv: dict[int, int], + items: Iterable[tuple[int, int]], + _on_dup: object, + ) -> tuple[dict[int, int], dict[int, int]]: + assert list(items) == [(3, 5), (6, 7)] + return {1: 2, 3: 5, 6: 7}, {2: 1, 5: 3, 7: 6} + + monkeypatch.setattr(base_mod, '_update_bidict_maps', fake_update) + + bi.update([(3, 5), (6, 7)]) + + assert dict(inv.items()) == {2: 1, 5: 3, 7: 6} + assert inv.inverse is bi + + native_build = native_mod.build_bidict_maps -if native_build is not None: +native_update = native_mod.update_bidict_maps +if native_build is not None or native_update is not None: pytest.importorskip('bidict_base_opt_native') @@ -87,3 +166,30 @@ def test_native_build_bidict_maps_raises_key_and_value_duplication_error() -> No assert native_build is not None with pytest.raises(KeyAndValueDuplicationError): native_build([(1, 2), (3, 4), (1, 4)], bidict.on_dup) + + +@pytest.mark.skipif(native_update is None, reason='optional native helper is not installed') +def test_native_update_bidict_maps_matches_drop_old_behavior() -> None: + assert native_update is not None + fwd = {1: 2, 3: 4} + inv = {2: 1, 4: 3} + + new_fwd, new_inv = native_update(fwd, inv, [(3, 5), (6, 5)], ON_DUP_DROP_OLD) + + assert new_fwd == {1: 2, 6: 5} + assert new_inv == {2: 1, 5: 6} + assert fwd == {1: 2, 3: 4} + assert inv == {2: 1, 4: 3} + + +@pytest.mark.skipif(native_update is None, reason='optional native helper is not installed') +def test_native_update_bidict_maps_raises_without_mutating_inputs() -> None: + assert native_update is not None + fwd = {1: 2, 3: 4} + inv = {2: 1, 4: 3} + + with pytest.raises(ValueDuplicationError): + native_update(fwd, inv, [(5, 6), (7, 4)], bidict.on_dup) + + assert fwd == {1: 2, 3: 4} + assert inv == {2: 1, 4: 3} From 8cc94233c2c79203af3ba44cc6d4eef18dc54f21 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 20:36:08 -0400 Subject: [PATCH 04/18] Add native helper env toggle Allow the optional native helper import to be disabled via BIDICT_DISABLE_NATIVE so benchmarking and debugging can easily force the pure-Python path. Add coverage for the env-var gate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- bidict/_native.py | 50 +++++++++++++++++++++++++++----------------- tests/test_native.py | 12 +++++++++++ 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/bidict/_native.py b/bidict/_native.py index e2d65bb8..5cacc974 100644 --- a/bidict/_native.py +++ b/bidict/_native.py @@ -8,12 +8,15 @@ from __future__ import annotations +import os import typing as t from collections.abc import Iterable from ._dup import OnDup +_DISABLE_NATIVE_ENVVAR = 'BIDICT_DISABLE_NATIVE' +_DISABLE_NATIVE_TRUE_VALUES = frozenset({'1', 'true', 'yes'}) Items = Iterable[tuple[t.Any, t.Any]] BuildBidictMaps: t.TypeAlias = t.Callable[[Items, OnDup], tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]] UpdateBidictMaps: t.TypeAlias = t.Callable[ @@ -24,6 +27,11 @@ update_bidict_maps: UpdateBidictMaps | None +def _native_disabled() -> bool: + value = os.getenv(_DISABLE_NATIVE_ENVVAR) + return value is not None and value.lower() in _DISABLE_NATIVE_TRUE_VALUES + + if t.TYPE_CHECKING: build_bidict_maps = None update_bidict_maps = None @@ -43,28 +51,32 @@ def _update_bidict_maps_impl( ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... else: - try: - from bidict_base_opt_native import bidict_base_opt_native as _native_ext - except ModuleNotFoundError as exc: - if exc.name != 'bidict_base_opt_native': - raise + if _native_disabled(): build_bidict_maps: BuildBidictMaps | None = None update_bidict_maps: UpdateBidictMaps | None = None else: - _build_bidict_maps_impl = _native_ext.build_bidict_maps - _update_bidict_maps_impl = getattr(_native_ext, 'update_bidict_maps', None) - - def build_bidict_maps(items: Items, on_dup: OnDup) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: - return _build_bidict_maps_impl(items, on_dup.key.name, on_dup.val.name) - - if _update_bidict_maps_impl is None: + try: + from bidict_base_opt_native import bidict_base_opt_native as _native_ext + except ModuleNotFoundError as exc: + if exc.name != 'bidict_base_opt_native': + raise + build_bidict_maps = None update_bidict_maps = None else: + _build_bidict_maps_impl = _native_ext.build_bidict_maps + _update_bidict_maps_impl = getattr(_native_ext, 'update_bidict_maps', None) + + def build_bidict_maps(items: Items, on_dup: OnDup) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: + return _build_bidict_maps_impl(items, on_dup.key.name, on_dup.val.name) + + if _update_bidict_maps_impl is None: + update_bidict_maps = None + else: - def update_bidict_maps( - fwd: dict[t.Any, t.Any], - inv: dict[t.Any, t.Any], - items: Items, - on_dup: OnDup, - ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: - return _update_bidict_maps_impl(fwd, inv, items, on_dup.key.name, on_dup.val.name) + def update_bidict_maps( + fwd: dict[t.Any, t.Any], + inv: dict[t.Any, t.Any], + items: Items, + on_dup: OnDup, + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: + return _update_bidict_maps_impl(fwd, inv, items, on_dup.key.name, on_dup.val.name) diff --git a/tests/test_native.py b/tests/test_native.py index 76d72191..dbab10e1 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -6,6 +6,7 @@ from __future__ import annotations +import importlib from collections.abc import Iterable import pytest @@ -20,6 +21,17 @@ from bidict import bidict +def test_native_env_var_disables_helpers(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv('BIDICT_DISABLE_NATIVE', '1') + reloaded = importlib.reload(native_mod) + try: + assert reloaded.build_bidict_maps is None + assert reloaded.update_bidict_maps is None + finally: + monkeypatch.delenv('BIDICT_DISABLE_NATIVE', raising=False) + importlib.reload(reloaded) + + def test_empty_update_uses_native_builder_when_available(monkeypatch: pytest.MonkeyPatch) -> None: items_seen: list[tuple[int, int]] = [] From 5c94a914613f2f2f9f036f7fe4bc5e71c81df8d2 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 20:51:13 -0400 Subject: [PATCH 05/18] Optimize native bulk update routing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- bidict/_base.py | 53 +++++++++++++++++++++++++++++++--------- tests/test_native.py | 58 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 11 deletions(-) diff --git a/bidict/_base.py b/bidict/_base.py index d07e70f6..b72766f9 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -57,6 +57,9 @@ DedupResult: t.TypeAlias = OldKV[KT, VT] | None Unwrites: t.TypeAlias = list[tuple[t.Any, ...]] ReversedIter: t.TypeAlias = t.Callable[['BidictBase[KT, t.Any]'], Iterator[KT]] +_MIN_NATIVE_UPDATE_ITEMS = 8192 +_MIN_NATIVE_FORCEUPDATE_ITEMS = 4096 +_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS = 4096 class BidictKeysView(KeysView[KT], ValuesView[KT]): @@ -224,6 +227,39 @@ def _set_map_data(self, fwdm: MutableMapping[KT, VT], invm: MutableMapping[VT, K self._fwdm.update(fwdm) self._invm.update(invm) + def _supports_native_map_swap(self) -> bool: + if type(self)._write is not BidictBase._write: + return False + return self._fwdm_cls is dict and self._invm_cls is dict + + def _should_use_native_update(self, incoming_len: int | None, on_dup: OnDup) -> bool: + if incoming_len is None or not self: + return False + if not self._supports_native_map_swap() or _update_bidict_maps is None: + return False + min_items = _MIN_NATIVE_FORCEUPDATE_ITEMS if on_dup.val is DROP_OLD else _MIN_NATIVE_UPDATE_ITEMS + return incoming_len >= min(len(self), min_items) + + def _maybe_prescan_native_update( + self, arg: MapOrItems[KT, VT], kw: Mapping[str, VT], on_dup: OnDup, incoming_len: int | None + ) -> None: + if ( + kw + or incoming_len is None + or incoming_len < _MIN_NATIVE_DUPVAL_PRESCAN_ITEMS + or not isinstance(arg, Mapping) + or on_dup.val is not RAISE + ): + return + seen_by_val: dict[VT, KT] = {} + seen_get = seen_by_val.get + for key, val in arg.items(): + prev_key = seen_get(val, MISSING) + if prev_key is MISSING: + seen_by_val[val] = key + elif prev_key != key: + raise ValueDuplicationError(val) + @property def inv(self) -> BidictBase[VT, KT]: """Alias for :attr:`inverse`.""" @@ -463,22 +499,17 @@ def _update( self._init_from(arg) return - if not self and self._fwdm_cls is dict and self._invm_cls is dict and _build_bidict_maps is not None: + if not self and self._supports_native_map_swap() and _build_bidict_maps is not None: self._set_map_data(*_build_bidict_maps(iteritems(arg, **kw), on_dup)) return - if ( - self - and rollback - and incoming_len is not None - and incoming_len >= len(self) - and self._fwdm_cls is dict - and self._invm_cls is dict - and _update_bidict_maps is not None - ): + if self._should_use_native_update(incoming_len, on_dup): + self._maybe_prescan_native_update(arg, kw, on_dup, incoming_len) fwdm = t.cast(dict[t.Any, t.Any], self._fwdm) invm = t.cast(dict[t.Any, t.Any], self._invm) - self._set_map_data(*_update_bidict_maps(fwdm, invm, iteritems(arg, **kw), on_dup)) + native_update = _update_bidict_maps + assert native_update is not None + self._set_map_data(*native_update(fwdm, invm, iteritems(arg, **kw), on_dup)) return # Fast path when we're adding more items than we contain already and rollback is enabled: diff --git a/tests/test_native.py b/tests/test_native.py index dbab10e1..04fa4dea 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -15,6 +15,7 @@ from bidict import ON_DUP_DROP_OLD from bidict import KeyAndValueDuplicationError from bidict import OnDup +from bidict import OrderedBidict from bidict import ValueDuplicationError from bidict import _base as base_mod from bidict import _native as native_mod @@ -81,6 +82,13 @@ def fail_build(_items: object, _on_dup: object) -> tuple[dict[int, int], dict[in assert dict(bi.items()) == {1: 2, 3: 4} +def test_orderedbidict_skips_native_builder() -> None: + bi = OrderedBidict([(1, 2), (3, 4)]) + + assert not bi._supports_native_map_swap() + assert tuple(bi.items()) == ((1, 2), (3, 4)) + + def test_nonempty_bulk_update_uses_native_updater_when_available(monkeypatch: pytest.MonkeyPatch) -> None: items_seen: list[tuple[int, int]] = [] @@ -96,6 +104,7 @@ def fake_update( return {1: 2, 3: 4, 5: 6, 7: 8}, {2: 1, 4: 3, 6: 5, 8: 7} monkeypatch.setattr(base_mod, '_update_bidict_maps', fake_update) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_FORCEUPDATE_ITEMS', 2) bi = bidict({1: 2, 3: 4}) bi.update([(5, 6), (7, 8)]) @@ -120,6 +129,54 @@ def fail_update( assert dict(bi.items()) == {1: 2, 3: 4, 5: 6} +def test_orderedbidict_skips_native_updater() -> None: + bi = OrderedBidict({1: 2, 3: 4}) + assert not bi._supports_native_map_swap() + + +def test_forceupdate_uses_native_updater_when_available(monkeypatch: pytest.MonkeyPatch) -> None: + items_seen: list[tuple[int, int]] = [] + + def fake_update( + fwd: dict[int, int], + inv: dict[int, int], + items: Iterable[tuple[int, int]], + on_dup: object, + ) -> tuple[dict[int, int], dict[int, int]]: + items_seen.extend(items) + assert fwd == {1: 2, 3: 4} + assert inv == {2: 1, 4: 3} + assert on_dup == ON_DUP_DROP_OLD + return {1: 2, 5: 4, 6: 7}, {2: 1, 4: 5, 7: 6} + + monkeypatch.setattr(base_mod, '_update_bidict_maps', fake_update) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 2) + bi = bidict({1: 2, 3: 4}) + + bi.forceupdate([(5, 4), (6, 7)]) + + assert items_seen == [(5, 4), (6, 7)] + assert dict(bi.items()) == {1: 2, 5: 4, 6: 7} + + +def test_large_mapping_dupval_failure_prescans_before_native_update(monkeypatch: pytest.MonkeyPatch) -> None: + def fail_update( + _fwd: object, _inv: object, _items: object, _on_dup: object + ) -> tuple[dict[int, int], dict[int, int]]: + msg = 'native updater should not run after duplicate-value prescan fails' + raise AssertionError(msg) + + monkeypatch.setattr(base_mod, '_update_bidict_maps', fail_update) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 1) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS', 2) + bi = bidict({10: 10}) + + with pytest.raises(ValueDuplicationError): + bi.update({1: 0, 2: 0}) + + assert dict(bi.items()) == {10: 10} + + def test_nonempty_native_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: bi = bidict({1: 2, 3: 4}) inv = bi.inverse @@ -134,6 +191,7 @@ def fake_update( return {1: 2, 3: 5, 6: 7}, {2: 1, 5: 3, 7: 6} monkeypatch.setattr(base_mod, '_update_bidict_maps', fake_update) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 2) bi.update([(3, 5), (6, 7)]) From 539ba952c6f255652b3c5492c2796925915a1723 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 21:06:59 -0400 Subject: [PATCH 06/18] Add native benchmark A/B coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- benchmark_linux_arm64.sh | 20 ++++++++++++++-- bidict/_base.py | 2 +- microbenchmarks.py | 50 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/benchmark_linux_arm64.sh b/benchmark_linux_arm64.sh index 18bee4ff..e150038c 100755 --- a/benchmark_linux_arm64.sh +++ b/benchmark_linux_arm64.sh @@ -20,12 +20,16 @@ repo_root=$( pwd ) +# shellcheck disable=SC2016 container run --rm --progress plain \ -v "${repo_root}:/work" \ -w /work \ "${container_image}" \ bash -c ' set -euo pipefail + if [ -n "${1:-}" ]; then + export BIDICT_DISABLE_NATIVE="$1" + fi export DEBIAN_FRONTEND=noninteractive export PYTHONHASHSEED=42 export UV_LINK_MODE=copy @@ -50,11 +54,23 @@ container run --rm --progress plain \ uv sync --all-groups --frozen >/dev/null . /tmp/bidict-bench-venv/bin/activate - python -c "import bidict._native as native; assert native.build_bidict_maps is not None; print(\"native helper:\", native.build_bidict_maps.__module__)" + python - <<"PY" +import os + +import bidict._native as native + +disabled = os.getenv("BIDICT_DISABLE_NATIVE") +if disabled: + assert native.build_bidict_maps is None and native.update_bidict_maps is None + print(f"native helper: disabled via BIDICT_DISABLE_NATIVE={disabled}") +else: + assert native.build_bidict_maps is not None + print("native helper:", native.build_bidict_maps.__module__) +PY ./cachegrind.py python -m pytest -c /dev/null -n0 \ --benchmark-columns=min,rounds,iterations \ --benchmark-disable-gc \ --benchmark-group-by=name \ microbenchmarks.py - ' + ' bash "${BIDICT_DISABLE_NATIVE-}" diff --git a/bidict/_base.py b/bidict/_base.py index b72766f9..8b9196d0 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -59,7 +59,7 @@ ReversedIter: t.TypeAlias = t.Callable[['BidictBase[KT, t.Any]'], Iterator[KT]] _MIN_NATIVE_UPDATE_ITEMS = 8192 _MIN_NATIVE_FORCEUPDATE_ITEMS = 4096 -_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS = 4096 +_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS = 16384 class BidictKeysView(KeysView[KT], ValuesView[KT]): diff --git a/microbenchmarks.py b/microbenchmarks.py index 8f908165..6a0f04c4 100644 --- a/microbenchmarks.py +++ b/microbenchmarks.py @@ -77,6 +77,10 @@ PARTIAL_OVERLAP_RESULTS_BY_LEN: dict[int, dict[int, int]] = { n: (INT_DICTS_BY_LEN[n] | PARTIAL_OVERLAP_UPDATES_BY_LEN[n]) for n in LENS } +FORCEUPDATE_EXISTING_VALUES_UPDATES_BY_LEN: dict[int, dict[int, int]] = {n: {n + i: i for i in range(n)} for n in LENS} +FORCEUPDATE_EXISTING_VALUES_RESULTS_BY_LEN: dict[int, dict[int, int]] = { + n: FORCEUPDATE_EXISTING_VALUES_UPDATES_BY_LEN[n].copy() for n in LENS +} BIDICT_AND_DICT_LAST_TWO_ITEMS_DIFFERENT_ORDER: dict[int, tuple[bidict.bidict[int, int], dict[int, int]]] = {} ORDERED_BIDICT_AND_DICT_LAST_TWO_ITEMS_DIFFERENT_ORDER: dict[ @@ -107,6 +111,10 @@ def _update(bi: bidict.bidict[int, int], other: dict[int, int], _expected: dict[ bi.update(other) +def _forceupdate(bi: bidict.bidict[int, int], other: dict[int, int], _expected: dict[int, int]) -> None: + bi.forceupdate(other) + + def _failing_update(bi: bidict.bidict[int, int], other: dict[int, int], _expected: dict[int, int]) -> None: with pytest.raises(bidict.DuplicationError): bi.update(other) @@ -173,6 +181,28 @@ def _setup_update_partial_overlap(n: int) -> tuple[tuple[t.Any, ...], dict[str, ) +def _setup_forceupdate_partial_overlap(n: int) -> tuple[tuple[t.Any, ...], dict[str, t.Any]]: + return ( + ( + INT_BIDICTS_BY_LEN[n].copy(), + PARTIAL_OVERLAP_UPDATES_BY_LEN[n], + PARTIAL_OVERLAP_RESULTS_BY_LEN[n], + ), + {}, + ) + + +def _setup_forceupdate_existing_values(n: int) -> tuple[tuple[t.Any, ...], dict[str, t.Any]]: + return ( + ( + INT_BIDICTS_BY_LEN[n].copy(), + FORCEUPDATE_EXISTING_VALUES_UPDATES_BY_LEN[n], + FORCEUPDATE_EXISTING_VALUES_RESULTS_BY_LEN[n], + ), + {}, + ) + + def _setup_failing_update_early(n: int) -> tuple[tuple[t.Any, ...], dict[str, t.Any]]: return ( ( @@ -305,6 +335,26 @@ def test_bi_update_partial_overlap(n: int, benchmark: t.Any) -> None: ) +@pytest.mark.parametrize('n', LENS) +def test_bi_forceupdate_partial_overlap(n: int, benchmark: t.Any) -> None: + """Benchmark forceupdating from a mapping with a mix of overlapping and new items.""" + benchmark.pedantic( + _forceupdate, + setup=lambda n=n: _setup_forceupdate_partial_overlap(n), + teardown=_assert_mapping_matches, + ) + + +@pytest.mark.parametrize('n', LENS) +def test_bi_forceupdate_existing_values(n: int, benchmark: t.Any) -> None: + """Benchmark forceupdating from a mapping whose values replace all existing ones.""" + benchmark.pedantic( + _forceupdate, + setup=lambda n=n: _setup_forceupdate_existing_values(n), + teardown=_assert_mapping_matches, + ) + + @pytest.mark.parametrize('n', LENS) def test_bi_update_fail_early_dupval(n: int, benchmark: t.Any) -> None: """Benchmark a bulk update that fails near the start and rolls back.""" From ca1b8cabbfb45e35f045fcce79c2a0d316cfc204 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 21:11:39 -0400 Subject: [PATCH 07/18] Add Rust pre-commit hooks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .pre-commit-config.yaml | 16 ++++++++++++++++ flake.nix | 6 +++--- init_dev_env | 2 +- rust/bidict_base_opt_native/src/lib.rs | 23 +++++++++++++---------- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fb8522f3..608e6a90 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -58,3 +58,19 @@ repos: hooks: - id: shellcheck exclude: ^.envrc$ + + - repo: local + hooks: + - id: cargo-fmt + name: cargo fmt + entry: cargo fmt --manifest-path rust/bidict_base_opt_native/Cargo.toml --check + language: system + files: ^rust/bidict_base_opt_native/ + pass_filenames: false + + - id: cargo-check + name: cargo check + entry: cargo check --manifest-path rust/bidict_base_opt_native/Cargo.toml --locked + language: system + files: ^rust/bidict_base_opt_native/ + pass_filenames: false diff --git a/flake.nix b/flake.nix index 4c63fbeb..8e7e7141 100644 --- a/flake.nix +++ b/flake.nix @@ -13,7 +13,7 @@ lib = pkgs.lib; latestPython = pkgs.python314; commonTools = with pkgs; [prek uv]; - nativeTools = with pkgs; [cargo rustc maturin]; + nativeTools = with pkgs; [cargo rustc rustfmt maturin]; supportedPythons = with pkgs; [ python314 python313 @@ -100,8 +100,8 @@ extraPackages = nativeTools; }; lint = pkgs.mkShell { - packages = with pkgs; [prek]; - shellHook = mkPathPrefix [pkgs.prek]; + packages = commonTools ++ nativeTools; + shellHook = mkPathPrefix (commonTools ++ nativeTools); }; test311 = mkTestShell { python = pkgs.python311; diff --git a/init_dev_env b/init_dev_env index 536c6616..731d8423 100755 --- a/init_dev_env +++ b/init_dev_env @@ -12,7 +12,7 @@ set -euo pipefail declare -r hint="Hint: Use 'nix develop' to bootstrap a development environment" -for cmd in uv prek rustc cargo maturin; do +for cmd in uv prek rustc cargo rustfmt maturin; do if ! command -v "$cmd" >/dev/null 2>&1; then >&2 echo "Error: No '$cmd' on PATH. $hint" exit 1 diff --git a/rust/bidict_base_opt_native/src/lib.rs b/rust/bidict_base_opt_native/src/lib.rs index 1c335383..c9cb3b42 100644 --- a/rust/bidict_base_opt_native/src/lib.rs +++ b/rust/bidict_base_opt_native/src/lib.rs @@ -2,7 +2,6 @@ use pyo3::prelude::*; use pyo3::types::PyDict; use pyo3::types::PyType; - #[derive(Clone, Copy)] enum OnDupAction { Raise, @@ -10,19 +9,19 @@ enum OnDupAction { DropNew, } - impl OnDupAction { fn parse(value: &str) -> PyResult { match value { "RAISE" => Ok(Self::Raise), "DROP_OLD" => Ok(Self::DropOld), "DROP_NEW" => Ok(Self::DropNew), - _ => Err(pyo3::exceptions::PyValueError::new_err(format!("unknown OnDupAction: {value}"))), + _ => Err(pyo3::exceptions::PyValueError::new_err(format!( + "unknown OnDupAction: {value}" + ))), } } } - fn bidict_err(py: Python<'_>, name: &str, args: A) -> PyErr where A: pyo3::PyErrArguments + Send + Sync + 'static, @@ -38,7 +37,6 @@ where PyErr::from_type_bound(err_type, args) } - fn apply_items( py: Python<'_>, fwd: &Bound<'_, PyDict>, @@ -64,7 +62,11 @@ fn apply_items( } match on_dup_val { OnDupAction::Raise => { - return Err(bidict_err(py, "KeyAndValueDuplicationError", (key.clone_ref(py), val.clone_ref(py)))); + return Err(bidict_err( + py, + "KeyAndValueDuplicationError", + (key.clone_ref(py), val.clone_ref(py)), + )); } OnDupAction::DropNew => continue, OnDupAction::DropOld => {} @@ -80,7 +82,11 @@ fn apply_items( } else if isdupval { match on_dup_val { OnDupAction::Raise => { - return Err(bidict_err(py, "ValueDuplicationError", (val.clone_ref(py),))); + return Err(bidict_err( + py, + "ValueDuplicationError", + (val.clone_ref(py),), + )); } OnDupAction::DropNew => continue, OnDupAction::DropOld => {} @@ -107,7 +113,6 @@ fn apply_items( Ok(()) } - #[pyfunction] fn build_bidict_maps( py: Python<'_>, @@ -125,7 +130,6 @@ fn build_bidict_maps( Ok((fwd.unbind(), inv.unbind())) } - #[pyfunction] fn update_bidict_maps( py: Python<'_>, @@ -145,7 +149,6 @@ fn update_bidict_maps( Ok((new_fwd.unbind(), new_inv.unbind())) } - #[pymodule] fn bidict_base_opt_native(_py: Python<'_>, module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_function(wrap_pyfunction!(build_bidict_maps, module)?)?; From d810aeb02a205719de092455533e89f166b003c6 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 21:16:34 -0400 Subject: [PATCH 08/18] Optimize native mapping handoff Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- bidict/_base.py | 10 ++++++++-- tests/test_native.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/bidict/_base.py b/bidict/_base.py index 8b9196d0..8c663d27 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -62,6 +62,12 @@ _MIN_NATIVE_DUPVAL_PRESCAN_ITEMS = 16384 +def _native_items(arg: MapOrItems[KT, VT], kw: Mapping[str, VT]) -> Iterable[tuple[KT, VT]]: + if not kw and isinstance(arg, Mapping): + return arg.items() + return iteritems(arg, **kw) + + class BidictKeysView(KeysView[KT], ValuesView[KT]): """Since the keys of a bidict are the values of its inverse (and vice versa), the :class:`~collections.abc.ValuesView` result of calling *bi.values()* @@ -500,7 +506,7 @@ def _update( return if not self and self._supports_native_map_swap() and _build_bidict_maps is not None: - self._set_map_data(*_build_bidict_maps(iteritems(arg, **kw), on_dup)) + self._set_map_data(*_build_bidict_maps(_native_items(arg, kw), on_dup)) return if self._should_use_native_update(incoming_len, on_dup): @@ -509,7 +515,7 @@ def _update( invm = t.cast(dict[t.Any, t.Any], self._invm) native_update = _update_bidict_maps assert native_update is not None - self._set_map_data(*native_update(fwdm, invm, iteritems(arg, **kw), on_dup)) + self._set_map_data(*native_update(fwdm, invm, _native_items(arg, kw), on_dup)) return # Fast path when we're adding more items than we contain already and rollback is enabled: diff --git a/tests/test_native.py b/tests/test_native.py index 04fa4dea..a9b5a838 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -50,6 +50,22 @@ def fake_build(items: Iterable[tuple[int, int]], _on_dup: object) -> tuple[dict[ assert dict(bi.inverse.items()) == {2: 1} +def test_empty_mapping_update_passes_items_view_to_native_builder(monkeypatch: pytest.MonkeyPatch) -> None: + seen_type: type[object] | None = None + + def fake_build(items: Iterable[tuple[int, int]], _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: + nonlocal seen_type + seen_type = type(items) + return {1: 2}, {2: 1} + + monkeypatch.setattr(base_mod, '_build_bidict_maps', fake_build) + bi = bidict[int, int]() + + bi.update({1: 2}) + + assert seen_type is type({}.items()) + + def test_empty_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: items_seen: list[tuple[int, int]] = [] bi = bidict[int, int]() @@ -113,6 +129,28 @@ def fake_update( assert dict(bi.items()) == {1: 2, 3: 4, 5: 6, 7: 8} +def test_nonempty_mapping_update_passes_items_view_to_native_updater(monkeypatch: pytest.MonkeyPatch) -> None: + seen_type: type[object] | None = None + + def fake_update( + _fwd: dict[int, int], + _inv: dict[int, int], + items: Iterable[tuple[int, int]], + _on_dup: object, + ) -> tuple[dict[int, int], dict[int, int]]: + nonlocal seen_type + seen_type = type(items) + return {1: 2, 3: 4, 5: 6, 7: 8}, {2: 1, 4: 3, 6: 5, 8: 7} + + monkeypatch.setattr(base_mod, '_update_bidict_maps', fake_update) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 2) + bi = bidict({1: 2, 3: 4}) + + bi.update({5: 6, 7: 8}) + + assert seen_type is type({}.items()) + + def test_nonempty_small_update_skips_native_updater(monkeypatch: pytest.MonkeyPatch) -> None: bi = bidict({1: 2, 3: 4}) From 7f073b413088cc7f7b3f302a637d13fa0b7d9dec Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 21:23:26 -0400 Subject: [PATCH 09/18] Add mapping-specialized native entrypoints Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- bidict/_base.py | 25 +++- bidict/_native.py | 53 +++++++ rust/bidict_base_opt_native/src/lib.rs | 200 ++++++++++++++++++------- tests/test_native.py | 85 ++++++++--- 4 files changed, 283 insertions(+), 80 deletions(-) diff --git a/bidict/_base.py b/bidict/_base.py index 8c663d27..484d8e5f 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -42,7 +42,9 @@ from ._iter import inverted from ._iter import iteritems from ._native import build_bidict_maps as _build_bidict_maps +from ._native import build_bidict_maps_from_mapping as _build_bidict_maps_from_mapping from ._native import update_bidict_maps as _update_bidict_maps +from ._native import update_bidict_maps_from_mapping as _update_bidict_maps_from_mapping from ._typing import KT from ._typing import MISSING from ._typing import OKT @@ -68,6 +70,10 @@ def _native_items(arg: MapOrItems[KT, VT], kw: Mapping[str, VT]) -> Iterable[tup return iteritems(arg, **kw) +def _supports_native_mapping(arg: MapOrItems[KT, VT], kw: Mapping[str, VT]) -> bool: + return not kw and isinstance(arg, Mapping) + + class BidictKeysView(KeysView[KT], ValuesView[KT]): """Since the keys of a bidict are the values of its inverse (and vice versa), the :class:`~collections.abc.ValuesView` result of calling *bi.values()* @@ -241,7 +247,9 @@ def _supports_native_map_swap(self) -> bool: def _should_use_native_update(self, incoming_len: int | None, on_dup: OnDup) -> bool: if incoming_len is None or not self: return False - if not self._supports_native_map_swap() or _update_bidict_maps is None: + if not self._supports_native_map_swap(): + return False + if _update_bidict_maps is None and _update_bidict_maps_from_mapping is None: return False min_items = _MIN_NATIVE_FORCEUPDATE_ITEMS if on_dup.val is DROP_OLD else _MIN_NATIVE_UPDATE_ITEMS return incoming_len >= min(len(self), min_items) @@ -505,14 +513,23 @@ def _update( self._init_from(arg) return - if not self and self._supports_native_map_swap() and _build_bidict_maps is not None: - self._set_map_data(*_build_bidict_maps(_native_items(arg, kw), on_dup)) - return + if not self and self._supports_native_map_swap(): + if _supports_native_mapping(arg, kw) and _build_bidict_maps_from_mapping is not None: + mapping_arg = t.cast(Mapping[t.Any, t.Any], arg) + self._set_map_data(*_build_bidict_maps_from_mapping(mapping_arg, on_dup)) + return + if _build_bidict_maps is not None: + self._set_map_data(*_build_bidict_maps(_native_items(arg, kw), on_dup)) + return if self._should_use_native_update(incoming_len, on_dup): self._maybe_prescan_native_update(arg, kw, on_dup, incoming_len) fwdm = t.cast(dict[t.Any, t.Any], self._fwdm) invm = t.cast(dict[t.Any, t.Any], self._invm) + if _supports_native_mapping(arg, kw) and _update_bidict_maps_from_mapping is not None: + mapping_arg = t.cast(Mapping[t.Any, t.Any], arg) + self._set_map_data(*_update_bidict_maps_from_mapping(fwdm, invm, mapping_arg, on_dup)) + return native_update = _update_bidict_maps assert native_update is not None self._set_map_data(*native_update(fwdm, invm, _native_items(arg, kw), on_dup)) diff --git a/bidict/_native.py b/bidict/_native.py index 5cacc974..70c09b85 100644 --- a/bidict/_native.py +++ b/bidict/_native.py @@ -11,6 +11,7 @@ import os import typing as t from collections.abc import Iterable +from collections.abc import Mapping from ._dup import OnDup @@ -23,8 +24,17 @@ [dict[t.Any, t.Any], dict[t.Any, t.Any], Items, OnDup], tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]], ] +BuildBidictMapsFromMapping: t.TypeAlias = t.Callable[ + [Mapping[t.Any, t.Any], OnDup], tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]] +] +UpdateBidictMapsFromMapping: t.TypeAlias = t.Callable[ + [dict[t.Any, t.Any], dict[t.Any, t.Any], Mapping[t.Any, t.Any], OnDup], + tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]], +] build_bidict_maps: BuildBidictMaps | None update_bidict_maps: UpdateBidictMaps | None +build_bidict_maps_from_mapping: BuildBidictMapsFromMapping | None +update_bidict_maps_from_mapping: UpdateBidictMapsFromMapping | None def _native_disabled() -> bool: @@ -35,6 +45,8 @@ def _native_disabled() -> bool: if t.TYPE_CHECKING: build_bidict_maps = None update_bidict_maps = None + build_bidict_maps_from_mapping = None + update_bidict_maps_from_mapping = None def _build_bidict_maps_impl( items: Items, @@ -50,10 +62,26 @@ def _update_bidict_maps_impl( on_dup_val: str, ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... + def _build_bidict_maps_from_mapping_impl( + mapping: Mapping[t.Any, t.Any], + on_dup_key: str, + on_dup_val: str, + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... + + def _update_bidict_maps_from_mapping_impl( + fwd: dict[t.Any, t.Any], + inv: dict[t.Any, t.Any], + mapping: Mapping[t.Any, t.Any], + on_dup_key: str, + on_dup_val: str, + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... + else: if _native_disabled(): build_bidict_maps: BuildBidictMaps | None = None update_bidict_maps: UpdateBidictMaps | None = None + build_bidict_maps_from_mapping: BuildBidictMapsFromMapping | None = None + update_bidict_maps_from_mapping: UpdateBidictMapsFromMapping | None = None else: try: from bidict_base_opt_native import bidict_base_opt_native as _native_ext @@ -62,13 +90,26 @@ def _update_bidict_maps_impl( raise build_bidict_maps = None update_bidict_maps = None + build_bidict_maps_from_mapping = None + update_bidict_maps_from_mapping = None else: _build_bidict_maps_impl = _native_ext.build_bidict_maps _update_bidict_maps_impl = getattr(_native_ext, 'update_bidict_maps', None) + _build_bidict_maps_from_mapping_impl = getattr(_native_ext, 'build_bidict_maps_from_mapping', None) + _update_bidict_maps_from_mapping_impl = getattr(_native_ext, 'update_bidict_maps_from_mapping', None) def build_bidict_maps(items: Items, on_dup: OnDup) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: return _build_bidict_maps_impl(items, on_dup.key.name, on_dup.val.name) + if _build_bidict_maps_from_mapping_impl is None: + build_bidict_maps_from_mapping = None + else: + + def build_bidict_maps_from_mapping( + mapping: Mapping[t.Any, t.Any], on_dup: OnDup + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: + return _build_bidict_maps_from_mapping_impl(mapping, on_dup.key.name, on_dup.val.name) + if _update_bidict_maps_impl is None: update_bidict_maps = None else: @@ -80,3 +121,15 @@ def update_bidict_maps( on_dup: OnDup, ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: return _update_bidict_maps_impl(fwd, inv, items, on_dup.key.name, on_dup.val.name) + + if _update_bidict_maps_from_mapping_impl is None: + update_bidict_maps_from_mapping = None + else: + + def update_bidict_maps_from_mapping( + fwd: dict[t.Any, t.Any], + inv: dict[t.Any, t.Any], + mapping: Mapping[t.Any, t.Any], + on_dup: OnDup, + ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: + return _update_bidict_maps_from_mapping_impl(fwd, inv, mapping, on_dup.key.name, on_dup.val.name) diff --git a/rust/bidict_base_opt_native/src/lib.rs b/rust/bidict_base_opt_native/src/lib.rs index c9cb3b42..0d66cdf1 100644 --- a/rust/bidict_base_opt_native/src/lib.rs +++ b/rust/bidict_base_opt_native/src/lib.rs @@ -37,6 +37,83 @@ where PyErr::from_type_bound(err_type, args) } +fn apply_item( + py: Python<'_>, + fwd: &Bound<'_, PyDict>, + inv: &Bound<'_, PyDict>, + key: &Bound<'_, PyAny>, + val: &Bound<'_, PyAny>, + on_dup_key: OnDupAction, + on_dup_val: OnDupAction, +) -> PyResult<()> { + let oldval = fwd.get_item(key)?.map(Bound::unbind); + let oldkey = inv.get_item(val)?.map(Bound::unbind); + let isdupkey = oldval.is_some(); + let isdupval = oldkey.is_some(); + + if isdupkey && isdupval { + let oldkey = oldkey.as_ref().expect("checked is_some above"); + let oldval = oldval.as_ref().expect("checked is_some above"); + if key.eq(oldkey.bind(py))? { + assert!(val.eq(oldval.bind(py))?); + return Ok(()); + } + match on_dup_val { + OnDupAction::Raise => { + return Err(bidict_err( + py, + "KeyAndValueDuplicationError", + (key.clone().unbind(), val.clone().unbind()), + )); + } + OnDupAction::DropNew => return Ok(()), + OnDupAction::DropOld => {} + } + } else if isdupkey { + match on_dup_key { + OnDupAction::Raise => { + return Err(bidict_err( + py, + "KeyDuplicationError", + (key.clone().unbind(),), + )); + } + OnDupAction::DropNew => return Ok(()), + OnDupAction::DropOld => {} + } + } else if isdupval { + match on_dup_val { + OnDupAction::Raise => { + return Err(bidict_err( + py, + "ValueDuplicationError", + (val.clone().unbind(),), + )); + } + OnDupAction::DropNew => return Ok(()), + OnDupAction::DropOld => {} + } + } + + fwd.set_item(key, val)?; + inv.set_item(val, key)?; + + if isdupkey && isdupval { + let oldkey = oldkey.as_ref().expect("checked is_some above"); + let oldval = oldval.as_ref().expect("checked is_some above"); + fwd.del_item(oldkey.bind(py))?; + inv.del_item(oldval.bind(py))?; + } else if isdupkey { + let oldval = oldval.as_ref().expect("checked is_some above"); + inv.del_item(oldval.bind(py))?; + } else if isdupval { + let oldkey = oldkey.as_ref().expect("checked is_some above"); + fwd.del_item(oldkey.bind(py))?; + } + + Ok(()) +} + fn apply_items( py: Python<'_>, fwd: &Bound<'_, PyDict>, @@ -48,66 +125,35 @@ fn apply_items( for item in items.iter()? { let item = item?; let (key, val): (Py, Py) = item.extract()?; - let oldval = fwd.get_item(key.bind(py))?.map(Bound::unbind); - let oldkey = inv.get_item(val.bind(py))?.map(Bound::unbind); - let isdupkey = oldval.is_some(); - let isdupval = oldkey.is_some(); - - if isdupkey && isdupval { - let oldkey = oldkey.as_ref().expect("checked is_some above"); - let oldval = oldval.as_ref().expect("checked is_some above"); - if key.bind(py).eq(oldkey.bind(py))? { - assert!(val.bind(py).eq(oldval.bind(py))?); - continue; - } - match on_dup_val { - OnDupAction::Raise => { - return Err(bidict_err( - py, - "KeyAndValueDuplicationError", - (key.clone_ref(py), val.clone_ref(py)), - )); - } - OnDupAction::DropNew => continue, - OnDupAction::DropOld => {} - } - } else if isdupkey { - match on_dup_key { - OnDupAction::Raise => { - return Err(bidict_err(py, "KeyDuplicationError", (key.clone_ref(py),))); - } - OnDupAction::DropNew => continue, - OnDupAction::DropOld => {} - } - } else if isdupval { - match on_dup_val { - OnDupAction::Raise => { - return Err(bidict_err( - py, - "ValueDuplicationError", - (val.clone_ref(py),), - )); - } - OnDupAction::DropNew => continue, - OnDupAction::DropOld => {} - } - } + apply_item( + py, + fwd, + inv, + &key.bind(py), + &val.bind(py), + on_dup_key, + on_dup_val, + )?; + } + + Ok(()) +} - fwd.set_item(key.bind(py), val.bind(py))?; - inv.set_item(val.bind(py), key.bind(py))?; - - if isdupkey && isdupval { - let oldkey = oldkey.as_ref().expect("checked is_some above"); - let oldval = oldval.as_ref().expect("checked is_some above"); - fwd.del_item(oldkey.bind(py))?; - inv.del_item(oldval.bind(py))?; - } else if isdupkey { - let oldval = oldval.as_ref().expect("checked is_some above"); - inv.del_item(oldval.bind(py))?; - } else if isdupval { - let oldkey = oldkey.as_ref().expect("checked is_some above"); - fwd.del_item(oldkey.bind(py))?; +fn apply_mapping( + py: Python<'_>, + fwd: &Bound<'_, PyDict>, + inv: &Bound<'_, PyDict>, + mapping: Bound<'_, PyAny>, + on_dup_key: OnDupAction, + on_dup_val: OnDupAction, +) -> PyResult<()> { + if let Ok(dict) = mapping.downcast::() { + for (key, val) in dict.iter() { + apply_item(py, fwd, inv, &key, &val, on_dup_key, on_dup_val)?; } + } else { + let items = mapping.call_method0("items")?; + apply_items(py, fwd, inv, items, on_dup_key, on_dup_val)?; } Ok(()) @@ -130,6 +176,23 @@ fn build_bidict_maps( Ok((fwd.unbind(), inv.unbind())) } +#[pyfunction] +fn build_bidict_maps_from_mapping( + py: Python<'_>, + mapping: Bound<'_, PyAny>, + on_dup_key: &str, + on_dup_val: &str, +) -> PyResult<(Py, Py)> { + let on_dup_key = OnDupAction::parse(on_dup_key)?; + let on_dup_val = OnDupAction::parse(on_dup_val)?; + let fwd = PyDict::new_bound(py); + let inv = PyDict::new_bound(py); + + apply_mapping(py, &fwd, &inv, mapping, on_dup_key, on_dup_val)?; + + Ok((fwd.unbind(), inv.unbind())) +} + #[pyfunction] fn update_bidict_maps( py: Python<'_>, @@ -149,9 +212,30 @@ fn update_bidict_maps( Ok((new_fwd.unbind(), new_inv.unbind())) } +#[pyfunction] +fn update_bidict_maps_from_mapping( + py: Python<'_>, + fwd: Bound<'_, PyDict>, + inv: Bound<'_, PyDict>, + mapping: Bound<'_, PyAny>, + on_dup_key: &str, + on_dup_val: &str, +) -> PyResult<(Py, Py)> { + let on_dup_key = OnDupAction::parse(on_dup_key)?; + let on_dup_val = OnDupAction::parse(on_dup_val)?; + let new_fwd = fwd.copy()?; + let new_inv = inv.copy()?; + + apply_mapping(py, &new_fwd, &new_inv, mapping, on_dup_key, on_dup_val)?; + + Ok((new_fwd.unbind(), new_inv.unbind())) +} + #[pymodule] fn bidict_base_opt_native(_py: Python<'_>, module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_function(wrap_pyfunction!(build_bidict_maps, module)?)?; + module.add_function(wrap_pyfunction!(build_bidict_maps_from_mapping, module)?)?; module.add_function(wrap_pyfunction!(update_bidict_maps, module)?)?; + module.add_function(wrap_pyfunction!(update_bidict_maps_from_mapping, module)?)?; Ok(()) } diff --git a/tests/test_native.py b/tests/test_native.py index a9b5a838..0139bf0e 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -27,7 +27,9 @@ def test_native_env_var_disables_helpers(monkeypatch: pytest.MonkeyPatch) -> Non reloaded = importlib.reload(native_mod) try: assert reloaded.build_bidict_maps is None + assert reloaded.build_bidict_maps_from_mapping is None assert reloaded.update_bidict_maps is None + assert reloaded.update_bidict_maps_from_mapping is None finally: monkeypatch.delenv('BIDICT_DISABLE_NATIVE', raising=False) importlib.reload(reloaded) @@ -50,20 +52,26 @@ def fake_build(items: Iterable[tuple[int, int]], _on_dup: object) -> tuple[dict[ assert dict(bi.inverse.items()) == {2: 1} -def test_empty_mapping_update_passes_items_view_to_native_builder(monkeypatch: pytest.MonkeyPatch) -> None: - seen_type: type[object] | None = None +def test_empty_mapping_update_uses_mapping_native_builder_when_available(monkeypatch: pytest.MonkeyPatch) -> None: + seen_mapping: dict[int, int] | None = None - def fake_build(items: Iterable[tuple[int, int]], _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: - nonlocal seen_type - seen_type = type(items) + def fail_build(_items: object, _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: + msg = 'generic native builder should not run for mapping updates' + raise AssertionError(msg) + + def fake_build_from_mapping(mapping: dict[int, int], _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: + nonlocal seen_mapping + seen_mapping = mapping return {1: 2}, {2: 1} - monkeypatch.setattr(base_mod, '_build_bidict_maps', fake_build) + monkeypatch.setattr(base_mod, '_build_bidict_maps', fail_build) + monkeypatch.setattr(base_mod, '_build_bidict_maps_from_mapping', fake_build_from_mapping) bi = bidict[int, int]() - bi.update({1: 2}) + mapping = {1: 2} + bi.update(mapping) - assert seen_type is type({}.items()) + assert seen_mapping is mapping def test_empty_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: @@ -129,26 +137,37 @@ def fake_update( assert dict(bi.items()) == {1: 2, 3: 4, 5: 6, 7: 8} -def test_nonempty_mapping_update_passes_items_view_to_native_updater(monkeypatch: pytest.MonkeyPatch) -> None: - seen_type: type[object] | None = None +def test_nonempty_mapping_update_uses_mapping_native_updater(monkeypatch: pytest.MonkeyPatch) -> None: + seen_mapping: dict[int, int] | None = None - def fake_update( + def fail_update( _fwd: dict[int, int], _inv: dict[int, int], - items: Iterable[tuple[int, int]], + _items: Iterable[tuple[int, int]], + _on_dup: object, + ) -> tuple[dict[int, int], dict[int, int]]: + msg = 'generic native updater should not run for mapping updates' + raise AssertionError(msg) + + def fake_update_from_mapping( + _fwd: dict[int, int], + _inv: dict[int, int], + mapping: dict[int, int], _on_dup: object, ) -> tuple[dict[int, int], dict[int, int]]: - nonlocal seen_type - seen_type = type(items) + nonlocal seen_mapping + seen_mapping = mapping return {1: 2, 3: 4, 5: 6, 7: 8}, {2: 1, 4: 3, 6: 5, 8: 7} - monkeypatch.setattr(base_mod, '_update_bidict_maps', fake_update) + monkeypatch.setattr(base_mod, '_update_bidict_maps', fail_update) + monkeypatch.setattr(base_mod, '_update_bidict_maps_from_mapping', fake_update_from_mapping) monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 2) bi = bidict({1: 2, 3: 4}) - bi.update({5: 6, 7: 8}) + mapping = {5: 6, 7: 8} + bi.update(mapping) - assert seen_type is type({}.items()) + assert seen_mapping is mapping def test_nonempty_small_update_skips_native_updater(monkeypatch: pytest.MonkeyPatch) -> None: @@ -238,8 +257,15 @@ def fake_update( native_build = native_mod.build_bidict_maps +native_build_from_mapping = native_mod.build_bidict_maps_from_mapping native_update = native_mod.update_bidict_maps -if native_build is not None or native_update is not None: +native_update_from_mapping = native_mod.update_bidict_maps_from_mapping +if ( + native_build is not None + or native_build_from_mapping is not None + or native_update is not None + or native_update_from_mapping is not None +): pytest.importorskip('bidict_base_opt_native') @@ -276,6 +302,15 @@ def test_native_build_bidict_maps_raises_key_and_value_duplication_error() -> No native_build([(1, 2), (3, 4), (1, 4)], bidict.on_dup) +@pytest.mark.skipif(native_build_from_mapping is None, reason='optional native mapping helper is not installed') +def test_native_build_bidict_maps_from_mapping_matches_drop_old_behavior() -> None: + assert native_build_from_mapping is not None + fwd, inv = native_build_from_mapping({1: 3, 4: 3}, ON_DUP_DROP_OLD) + + assert fwd == {4: 3} + assert inv == {3: 4} + + @pytest.mark.skipif(native_update is None, reason='optional native helper is not installed') def test_native_update_bidict_maps_matches_drop_old_behavior() -> None: assert native_update is not None @@ -301,3 +336,17 @@ def test_native_update_bidict_maps_raises_without_mutating_inputs() -> None: assert fwd == {1: 2, 3: 4} assert inv == {2: 1, 4: 3} + + +@pytest.mark.skipif(native_update_from_mapping is None, reason='optional native mapping helper is not installed') +def test_native_update_bidict_maps_from_mapping_matches_drop_old_behavior() -> None: + assert native_update_from_mapping is not None + fwd = {1: 2, 3: 4} + inv = {2: 1, 4: 3} + + new_fwd, new_inv = native_update_from_mapping(fwd, inv, {3: 5, 6: 5}, ON_DUP_DROP_OLD) + + assert new_fwd == {1: 2, 6: 5} + assert new_inv == {2: 1, 5: 6} + assert fwd == {1: 2, 3: 4} + assert inv == {2: 1, 4: 3} From 3c556df8e98b86815c7f24a15f67292d1ef8da74 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Tue, 26 May 2026 21:37:13 -0400 Subject: [PATCH 10/18] Add hermetic clippy hook Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .pre-commit-config.yaml | 11 +++++++++++ CONTRIBUTING.rst | 3 ++- flake.nix | 17 +++++++++-------- init_dev_env | 2 +- rust/bidict_base_opt_native/src/lib.rs | 9 +++++++-- 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 608e6a90..ec88dee3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -74,3 +74,14 @@ repos: language: system files: ^rust/bidict_base_opt_native/ pass_filenames: false + + - id: cargo-clippy + name: cargo clippy + entry: >- + bash -c 'export RUSTC="$(command -v rustc)"; + export RUSTC_WORKSPACE_WRAPPER="$(command -v clippy-driver)"; + export RUSTFLAGS="-D warnings"; + cargo check --manifest-path rust/bidict_base_opt_native/Cargo.toml --locked --all-targets' + language: system + files: ^rust/bidict_base_opt_native/ + pass_filenames: false diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 64467fff..68db550e 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -45,7 +45,8 @@ Making Changes - If you have `Nix `__, run ``nix develop`` from within your clone to start a shell where all supported Python versions as well as ``prek``, ``uv``, ``rustc``, - ``cargo``, and ``maturin`` are installed and added to your PATH. + ``cargo``, ``rustfmt``, ``cargo-clippy``, and ``maturin`` + are installed and added to your PATH. This also pins ``uv`` to the flake-provided default Python when initializing ``.venv``. diff --git a/flake.nix b/flake.nix index 8e7e7141..a7cf1d4c 100644 --- a/flake.nix +++ b/flake.nix @@ -12,8 +12,9 @@ pkgs = import nixpkgs { inherit system; }; lib = pkgs.lib; latestPython = pkgs.python314; - commonTools = with pkgs; [prek uv]; - nativeTools = with pkgs; [cargo rustc rustfmt maturin]; + baseDevTools = with pkgs; [prek uv]; + rustDevTools = with pkgs; [cargo rustc rustfmt clippy maturin]; + allDevTools = baseDevTools ++ rustDevTools; supportedPythons = with pkgs; [ python314 python313 @@ -53,7 +54,7 @@ extraShellHook ? "", }: let - packages = commonTools ++ [python] ++ extraPackages; + packages = baseDevTools ++ [python] ++ extraPackages; in pkgs.mkShell { inherit packages; @@ -75,7 +76,7 @@ devShells = { default = let - packages = commonTools ++ nativeTools ++ supportedPythons; + packages = allDevTools ++ supportedPythons; in pkgs.mkShell { inherit packages; @@ -97,11 +98,11 @@ }; build = mkUvShell { python = pkgs.python313; - extraPackages = nativeTools; + extraPackages = rustDevTools; }; lint = pkgs.mkShell { - packages = commonTools ++ nativeTools; - shellHook = mkPathPrefix (commonTools ++ nativeTools); + packages = allDevTools; + shellHook = mkPathPrefix allDevTools; }; test311 = mkTestShell { python = pkgs.python311; @@ -125,7 +126,7 @@ }; update_deps = mkUvShell { python = latestPython; - extraPackages = nativeTools; + extraPackages = rustDevTools; }; }; }); diff --git a/init_dev_env b/init_dev_env index 731d8423..81778bfa 100755 --- a/init_dev_env +++ b/init_dev_env @@ -12,7 +12,7 @@ set -euo pipefail declare -r hint="Hint: Use 'nix develop' to bootstrap a development environment" -for cmd in uv prek rustc cargo rustfmt maturin; do +for cmd in uv prek rustc cargo rustfmt cargo-clippy clippy-driver maturin; do if ! command -v "$cmd" >/dev/null 2>&1; then >&2 echo "Error: No '$cmd' on PATH. $hint" exit 1 diff --git a/rust/bidict_base_opt_native/src/lib.rs b/rust/bidict_base_opt_native/src/lib.rs index 0d66cdf1..6e11b031 100644 --- a/rust/bidict_base_opt_native/src/lib.rs +++ b/rust/bidict_base_opt_native/src/lib.rs @@ -1,3 +1,8 @@ +#![allow( + clippy::useless_conversion, + reason = "PyO3 #[pyfunction] expansion triggers this false positive on PyResult returns" +)] + use pyo3::prelude::*; use pyo3::types::PyDict; use pyo3::types::PyType; @@ -129,8 +134,8 @@ fn apply_items( py, fwd, inv, - &key.bind(py), - &val.bind(py), + key.bind(py), + val.bind(py), on_dup_key, on_dup_val, )?; From 5f8a0cd4e4b7aec796076fe6330c0938ad6f6bb0 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Wed, 27 May 2026 10:14:48 -0400 Subject: [PATCH 11/18] Enable native helper in CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/benchmark.yml | 8 +++++++ .github/workflows/test.yml | 18 ++++++++++++++++ .../workflows/update_benchmark_baselines.yml | 8 +++++++ flake.nix | 21 ++++++++++++++++--- init_dev_env | 2 +- 5 files changed, 53 insertions(+), 4 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 450cdf30..82a8c225 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -72,6 +72,14 @@ jobs: git checkout ${{ github.sha }} # move aside the '"'"'bidict'"'"' subdirectory to make sure we always import the installed version mv -v bidict src + python - <<'"'"'PY'"'"' + from bidict import _native + + assert _native.build_bidict_maps is not None + assert _native.build_bidict_maps_from_mapping is not None + assert _native.update_bidict_maps is not None + assert _native.update_bidict_maps_from_mapping is not None + PY curl -L -s -o baseline.json "${{ steps.metadata.outputs.baseline_url }}" line1=$(head -n1 baseline.json) [ "$line1" = "{" ] diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c417f970..c5305fca 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,16 +36,21 @@ jobs: - python: "3.14" shell: test314 enable_coverage: true + enable_native: true - python: "3.13" shell: test313 + enable_native: true - python: "3.12" shell: test312 + enable_native: true - python: "3.11" shell: test311 # bidict/_typing.py's `typing`/`typing_extensions` imports branch on python<3.12 enable_coverage: true + enable_native: true - python: "pypy-3.11" shell: testPyPy311 + enable_native: false steps: - name: check out source uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd @@ -69,6 +74,19 @@ jobs: if: matrix.enable_coverage run: | echo RUN_PYTEST_CMD="coverage run -m pytest" >> "$GITHUB_ENV" + - name: verify native helper availability + if: matrix.enable_native + run: | + nix develop .#${{ matrix.shell }} --command bash -c ' + python - <<'"'"'PY'"'"' + from bidict import _native + + assert _native.build_bidict_maps is not None + assert _native.build_bidict_maps_from_mapping is not None + assert _native.update_bidict_maps is not None + assert _native.update_bidict_maps_from_mapping is not None + PY + ' - name: run mypy and pytest run: | nix develop .#${{ matrix.shell }} --command bash -c " diff --git a/.github/workflows/update_benchmark_baselines.yml b/.github/workflows/update_benchmark_baselines.yml index 7953af78..d5e6d2d7 100644 --- a/.github/workflows/update_benchmark_baselines.yml +++ b/.github/workflows/update_benchmark_baselines.yml @@ -59,6 +59,14 @@ jobs: uv pip install --python "$VIRTUAL_ENV/bin/python" --no-deps . # Move aside the working tree package so we always import the installed version. mv -v bidict src + python - <<'"'"'PY'"'"' + from bidict import _native + + assert _native.build_bidict_maps is not None + assert _native.build_bidict_maps_from_mapping is not None + assert _native.update_bidict_maps is not None + assert _native.update_bidict_maps_from_mapping is not None + PY ./cachegrind.py pytest -c /dev/null -n0 \ --benchmark-autosave \ --benchmark-columns=min,rounds,iterations \ diff --git a/flake.nix b/flake.nix index a7cf1d4c..50c90d07 100644 --- a/flake.nix +++ b/flake.nix @@ -15,6 +15,8 @@ baseDevTools = with pkgs; [prek uv]; rustDevTools = with pkgs; [cargo rustc rustfmt clippy maturin]; allDevTools = baseDevTools ++ rustDevTools; + nativePackageName = "bidict-base-opt-native"; + nativeReinstallArg = "--reinstall-package=${nativePackageName}"; supportedPythons = with pkgs; [ python314 python313 @@ -63,11 +65,19 @@ }; }; - mkTestShell = { python, projectEnv }: + mkTestShell = { + python, + projectEnv, + enableNative ? false, + }: mkUvShell { inherit python projectEnv; - syncArgs = "--only-group=test"; + syncArgs = + if enableNative + then "--only-group=test --only-group=native ${nativeReinstallArg}" + else "--only-group=test"; activate = true; + extraPackages = lib.optionals enableNative rustDevTools; extraShellHook = '' uv pip install --python "$UV_PROJECT_ENVIRONMENT/bin/python" --no-deps -e . ''; @@ -93,8 +103,9 @@ benchmark = mkUvShell { python = latestPython; projectEnv = ".venv-benchmark"; - syncArgs = "--only-group=test"; + syncArgs = "--only-group=test --only-group=native ${nativeReinstallArg}"; activate = true; + extraPackages = rustDevTools; }; build = mkUvShell { python = pkgs.python313; @@ -107,18 +118,22 @@ test311 = mkTestShell { python = pkgs.python311; projectEnv = ".venv-test-3.11"; + enableNative = true; }; test312 = mkTestShell { python = pkgs.python312; projectEnv = ".venv-test-3.12"; + enableNative = true; }; test313 = mkTestShell { python = pkgs.python313; projectEnv = ".venv-test-3.13"; + enableNative = true; }; test314 = mkTestShell { python = pkgs.python314; projectEnv = ".venv-test-3.14"; + enableNative = true; }; testPyPy311 = mkTestShell { python = pkgs.pypy3; diff --git a/init_dev_env b/init_dev_env index 81778bfa..7134ac23 100755 --- a/init_dev_env +++ b/init_dev_env @@ -20,5 +20,5 @@ for cmd in uv prek rustc cargo rustfmt cargo-clippy clippy-driver maturin; do done prek install -f -uv sync --all-groups +uv sync --all-groups --reinstall-package bidict-base-opt-native echo "Development virtualenv initialized" From 13a32ee0d488094f02481519e260ba6cc8d79650 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Wed, 27 May 2026 11:02:10 -0400 Subject: [PATCH 12/18] Optimize native item ingestion fast path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- rust/bidict_base_opt_native/src/lib.rs | 51 ++++++++++++++++++++------ 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/rust/bidict_base_opt_native/src/lib.rs b/rust/bidict_base_opt_native/src/lib.rs index 6e11b031..80d9f356 100644 --- a/rust/bidict_base_opt_native/src/lib.rs +++ b/rust/bidict_base_opt_native/src/lib.rs @@ -42,17 +42,22 @@ where PyErr::from_type_bound(err_type, args) } -fn apply_item( +struct ExistingItems { + oldval: Option>, + oldkey: Option>, +} + +fn handle_dup_item( py: Python<'_>, fwd: &Bound<'_, PyDict>, inv: &Bound<'_, PyDict>, key: &Bound<'_, PyAny>, val: &Bound<'_, PyAny>, - on_dup_key: OnDupAction, - on_dup_val: OnDupAction, + existing: ExistingItems, + on_dup: (OnDupAction, OnDupAction), ) -> PyResult<()> { - let oldval = fwd.get_item(key)?.map(Bound::unbind); - let oldkey = inv.get_item(val)?.map(Bound::unbind); + let ExistingItems { oldval, oldkey } = existing; + let (on_dup_key, on_dup_val) = on_dup; let isdupkey = oldval.is_some(); let isdupval = oldkey.is_some(); @@ -130,14 +135,23 @@ fn apply_items( for item in items.iter()? { let item = item?; let (key, val): (Py, Py) = item.extract()?; - apply_item( + let key = key.bind(py); + let val = val.bind(py); + let oldval = fwd.get_item(key)?.map(Bound::unbind); + let oldkey = inv.get_item(val)?.map(Bound::unbind); + if oldval.is_none() && oldkey.is_none() { + fwd.set_item(key, val)?; + inv.set_item(val, key)?; + continue; + } + handle_dup_item( py, fwd, inv, - key.bind(py), - val.bind(py), - on_dup_key, - on_dup_val, + key, + val, + ExistingItems { oldval, oldkey }, + (on_dup_key, on_dup_val), )?; } @@ -154,7 +168,22 @@ fn apply_mapping( ) -> PyResult<()> { if let Ok(dict) = mapping.downcast::() { for (key, val) in dict.iter() { - apply_item(py, fwd, inv, &key, &val, on_dup_key, on_dup_val)?; + let oldval = fwd.get_item(&key)?.map(Bound::unbind); + let oldkey = inv.get_item(&val)?.map(Bound::unbind); + if oldval.is_none() && oldkey.is_none() { + fwd.set_item(&key, &val)?; + inv.set_item(&val, &key)?; + continue; + } + handle_dup_item( + py, + fwd, + inv, + &key, + &val, + ExistingItems { oldval, oldkey }, + (on_dup_key, on_dup_val), + )?; } } else { let items = mapping.call_method0("items")?; From 37cf5cd9838a9d75c2632f43e8cecc1b446ecc34 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Wed, 27 May 2026 12:43:58 -0400 Subject: [PATCH 13/18] Show benchmark deltas in PR comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/benchmark.yml | 82 +++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 82a8c225..0df957af 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -19,6 +19,8 @@ env: PYTHONHASHSEED: "42" BASELINE_RELEASE_TAG: microbenchmarks PR_BENCHMARK_COMPARE_FAIL: min:5% + PR_BENCHMARK_COMMENT_MAX_ROWS: "8" + PR_BENCHMARK_COMMENT_MIN_CHANGE_PCT: "1" jobs: benchmark: @@ -88,6 +90,7 @@ jobs: compare_fail_args+=(--benchmark-compare-fail="$COMPARE_FAIL_EXPR") fi ./cachegrind.py pytest -c /dev/null -n0 \ + --benchmark-json benchmark.json \ --benchmark-autosave \ --benchmark-columns=min,rounds,iterations \ --benchmark-disable-gc \ @@ -138,6 +141,8 @@ jobs: if: always() && github.event_name == 'pull_request' env: BASELINE_ASSET_NAME: ${{ steps.metadata.outputs.baseline_asset_name }} + COMMENT_MAX_ROWS: ${{ env.PR_BENCHMARK_COMMENT_MAX_ROWS }} + COMMENT_MIN_CHANGE_PCT: ${{ env.PR_BENCHMARK_COMMENT_MIN_CHANGE_PCT }} PR_NUMBER: ${{ github.event.pull_request.number }} RESULT_MESSAGE: ${{ steps.benchmark.outputs.result_message }} RESULT_STATE: ${{ steps.benchmark.outputs.result_state }} @@ -173,6 +178,81 @@ jobs: import os from pathlib import Path + baseline_path = Path('baseline.json') + benchmark_path = Path('benchmark.json') + comment_path = Path('benchmark-pr-comment/comment.md') + max_rows = int(os.environ['COMMENT_MAX_ROWS']) + min_change_pct = float(os.environ['COMMENT_MIN_CHANGE_PCT']) + + def bench_key(bench: dict[str, object]) -> str: + return str(bench['name']) + + def load_benchmark_mins(path: Path) -> dict[str, float]: + data = json.loads(path.read_text()) + return {bench_key(bench): float(bench['stats']['min']) for bench in data['benchmarks']} + + def format_row(name: str, baseline: float, current: float, delta_pct: float) -> str: + return ( + f"| `{name}` | {baseline * 1000:.3f} ms | {current * 1000:.3f} ms | {delta_pct:+.2f}% |" + ) + + if baseline_path.exists() and benchmark_path.exists(): + baseline_mins = load_benchmark_mins(baseline_path) + benchmark_mins = load_benchmark_mins(benchmark_path) + shared = [] + for name, current in benchmark_mins.items(): + baseline = baseline_mins.get(name) + if baseline is None or baseline == 0: + continue + delta_pct = ((current - baseline) / baseline) * 100 + shared.append((name, baseline, current, delta_pct)) + + improvements = [ + entry for entry in shared if entry[3] <= -min_change_pct + ] + regressions = [ + entry for entry in shared if entry[3] >= min_change_pct + ] + improvements.sort(key=lambda entry: entry[3]) + regressions.sort(key=lambda entry: entry[3], reverse=True) + + lines = comment_path.read_text().rstrip().splitlines() + lines.extend(['', f'_Deltas below compare benchmark `min` time against `{os.environ["BASELINE_ASSET_NAME"]}`._']) + + if improvements: + lines.extend([ + '', + '### Notable improvements', + '', + '| Benchmark | Baseline | PR | Delta |', + '| --- | ---: | ---: | ---: |', + *( + format_row(name, baseline, current, delta_pct) + for name, baseline, current, delta_pct in improvements[:max_rows] + ), + ]) + + if regressions: + lines.extend([ + '', + '### Notable regressions', + '', + '| Benchmark | Baseline | PR | Delta |', + '| --- | ---: | ---: | ---: |', + *( + format_row(name, baseline, current, delta_pct) + for name, baseline, current, delta_pct in regressions[:max_rows] + ), + ]) + + if not improvements and not regressions: + lines.extend([ + '', + f'_No benchmark deltas of at least {min_change_pct:.0f}% to report._', + ]) + + comment_path.write_text('\n'.join(lines) + '\n') + payload = { 'pr_number': os.environ['PR_NUMBER'], 'result_state': os.environ['RESULT_STATE'], @@ -197,6 +277,8 @@ jobs: name: microbenchmark results (CPython ${{ steps.metadata.outputs.python_version }}) path: | .benchmarks + baseline.json + benchmark.json benchmark-output.txt include-hidden-files: true if-no-files-found: error From ca2887be6fc0bc654f20d16fd31719ff18010b93 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Wed, 27 May 2026 14:55:18 -0400 Subject: [PATCH 14/18] Fix workflow native smoke checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/benchmark.yml | 9 +-------- .github/workflows/test.yml | 9 +-------- .github/workflows/update_benchmark_baselines.yml | 9 +-------- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 0df957af..9664a21a 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -74,14 +74,7 @@ jobs: git checkout ${{ github.sha }} # move aside the '"'"'bidict'"'"' subdirectory to make sure we always import the installed version mv -v bidict src - python - <<'"'"'PY'"'"' - from bidict import _native - - assert _native.build_bidict_maps is not None - assert _native.build_bidict_maps_from_mapping is not None - assert _native.update_bidict_maps is not None - assert _native.update_bidict_maps_from_mapping is not None - PY + python -c "from bidict import _native; assert _native.build_bidict_maps is not None; assert _native.build_bidict_maps_from_mapping is not None; assert _native.update_bidict_maps is not None; assert _native.update_bidict_maps_from_mapping is not None" curl -L -s -o baseline.json "${{ steps.metadata.outputs.baseline_url }}" line1=$(head -n1 baseline.json) [ "$line1" = "{" ] diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c5305fca..4cfa73c7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -78,14 +78,7 @@ jobs: if: matrix.enable_native run: | nix develop .#${{ matrix.shell }} --command bash -c ' - python - <<'"'"'PY'"'"' - from bidict import _native - - assert _native.build_bidict_maps is not None - assert _native.build_bidict_maps_from_mapping is not None - assert _native.update_bidict_maps is not None - assert _native.update_bidict_maps_from_mapping is not None - PY + python -c "from bidict import _native; assert _native.build_bidict_maps is not None; assert _native.build_bidict_maps_from_mapping is not None; assert _native.update_bidict_maps is not None; assert _native.update_bidict_maps_from_mapping is not None" ' - name: run mypy and pytest run: | diff --git a/.github/workflows/update_benchmark_baselines.yml b/.github/workflows/update_benchmark_baselines.yml index d5e6d2d7..31c0cfc7 100644 --- a/.github/workflows/update_benchmark_baselines.yml +++ b/.github/workflows/update_benchmark_baselines.yml @@ -59,14 +59,7 @@ jobs: uv pip install --python "$VIRTUAL_ENV/bin/python" --no-deps . # Move aside the working tree package so we always import the installed version. mv -v bidict src - python - <<'"'"'PY'"'"' - from bidict import _native - - assert _native.build_bidict_maps is not None - assert _native.build_bidict_maps_from_mapping is not None - assert _native.update_bidict_maps is not None - assert _native.update_bidict_maps_from_mapping is not None - PY + python -c "from bidict import _native; assert _native.build_bidict_maps is not None; assert _native.build_bidict_maps_from_mapping is not None; assert _native.update_bidict_maps is not None; assert _native.update_bidict_maps_from_mapping is not None" ./cachegrind.py pytest -c /dev/null -n0 \ --benchmark-autosave \ --benchmark-columns=min,rounds,iterations \ From 1defc27bfdfa9014d9bad2ae70a6404392addc48 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Thu, 28 May 2026 11:05:35 -0400 Subject: [PATCH 15/18] Fix benchmark reporting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/benchmark.yml | 25 ++++++++++++++++++++-- cachegrind.py | 38 ++++++++++++++++++--------------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 9664a21a..8d5ee162 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -21,6 +21,8 @@ env: PR_BENCHMARK_COMPARE_FAIL: min:5% PR_BENCHMARK_COMMENT_MAX_ROWS: "8" PR_BENCHMARK_COMMENT_MIN_CHANGE_PCT: "1" + PR_BENCHMARK_COMMENT_MIN_BASELINE_MS: "1" + PR_BENCHMARK_COMMENT_MIN_DELTA_MS: "0.1" jobs: benchmark: @@ -136,6 +138,8 @@ jobs: BASELINE_ASSET_NAME: ${{ steps.metadata.outputs.baseline_asset_name }} COMMENT_MAX_ROWS: ${{ env.PR_BENCHMARK_COMMENT_MAX_ROWS }} COMMENT_MIN_CHANGE_PCT: ${{ env.PR_BENCHMARK_COMMENT_MIN_CHANGE_PCT }} + COMMENT_MIN_BASELINE_MS: ${{ env.PR_BENCHMARK_COMMENT_MIN_BASELINE_MS }} + COMMENT_MIN_DELTA_MS: ${{ env.PR_BENCHMARK_COMMENT_MIN_DELTA_MS }} PR_NUMBER: ${{ github.event.pull_request.number }} RESULT_MESSAGE: ${{ steps.benchmark.outputs.result_message }} RESULT_STATE: ${{ steps.benchmark.outputs.result_state }} @@ -176,6 +180,8 @@ jobs: comment_path = Path('benchmark-pr-comment/comment.md') max_rows = int(os.environ['COMMENT_MAX_ROWS']) min_change_pct = float(os.environ['COMMENT_MIN_CHANGE_PCT']) + min_baseline_ms = float(os.environ['COMMENT_MIN_BASELINE_MS']) + min_delta_ms = float(os.environ['COMMENT_MIN_DELTA_MS']) def bench_key(bench: dict[str, object]) -> str: return str(bench['name']) @@ -197,6 +203,10 @@ jobs: baseline = baseline_mins.get(name) if baseline is None or baseline == 0: continue + baseline_ms = baseline * 1000 + delta_ms = abs(current - baseline) * 1000 + if baseline_ms < min_baseline_ms or delta_ms < min_delta_ms: + continue delta_pct = ((current - baseline) / baseline) * 100 shared.append((name, baseline, current, delta_pct)) @@ -210,7 +220,14 @@ jobs: regressions.sort(key=lambda entry: entry[3], reverse=True) lines = comment_path.read_text().rstrip().splitlines() - lines.extend(['', f'_Deltas below compare benchmark `min` time against `{os.environ["BASELINE_ASSET_NAME"]}`._']) + lines.extend([ + '', + ( + f'_Deltas below compare benchmark `min` time against ' + f'`{os.environ["BASELINE_ASSET_NAME"]}` for benchmarks with baseline >= ' + f'{min_baseline_ms:g} ms and absolute delta >= {min_delta_ms:g} ms._' + ), + ]) if improvements: lines.extend([ @@ -241,7 +258,11 @@ jobs: if not improvements and not regressions: lines.extend([ '', - f'_No benchmark deltas of at least {min_change_pct:.0f}% to report._', + ( + f'_No benchmark deltas meeting the reporting thresholds ' + f'({min_change_pct:.0f}% change, {min_baseline_ms:g} ms baseline, ' + f'{min_delta_ms:g} ms absolute delta) to report._' + ), ]) comment_path.write_text('\n'.join(lines) + '\n') diff --git a/cachegrind.py b/cachegrind.py index 9524b427..db82e7aa 100755 --- a/cachegrind.py +++ b/cachegrind.py @@ -48,7 +48,7 @@ DISABLE_ASLR_CMD = ['setarch', ARCH, '-R'] -def run_with_cachegrind(args_list: list[str]) -> dict[str, int]: +def run_with_cachegrind(args_list: list[str]) -> tuple[dict[str, int], int]: """ Run the the given program and arguments under Cachegrind, parse the Cachegrind specs. @@ -57,21 +57,24 @@ def run_with_cachegrind(args_list: list[str]) -> dict[str, int]: """ temp_file = NamedTemporaryFile('r+') # noqa: SIM115 # Don't fail if the program fails (to support e.g. `pytest --benchmark-compare-fail=...`) - sp.call([ - *DISABLE_ASLR_CMD, - 'valgrind', - '--tool=cachegrind', - '--cache-sim=yes', - # Set some reasonable L1 and LL values, based on Haswell. - # Feel free to update, important part is that they are consistent across runs, - # instead of the default of copying from the current machine. - '--I1=32768,8,64', - '--D1=32768,8,64', - '--LL=8388608,16,64', - '--cachegrind-out-file=' + temp_file.name, - *args_list, - ]) - return parse_cachegrind_output(temp_file) + completed = sp.run( + [ + *DISABLE_ASLR_CMD, + 'valgrind', + '--tool=cachegrind', + '--cache-sim=yes', + # Set some reasonable L1 and LL values, based on Haswell. + # Feel free to update, important part is that they are consistent across runs, + # instead of the default of copying from the current machine. + '--I1=32768,8,64', + '--D1=32768,8,64', + '--LL=8388608,16,64', + '--cachegrind-out-file=' + temp_file.name, + *args_list, + ], + check=False, + ) + return parse_cachegrind_output(temp_file), completed.returncode def parse_cachegrind_output(temp_file: t.IO[str]) -> dict[str, int]: @@ -128,10 +131,11 @@ def combined_instruction_estimate(counts: dict[str, int]) -> int: def main() -> None: - results = run_with_cachegrind(sys.argv[1:]) + results, exit_code = run_with_cachegrind(sys.argv[1:]) counts = get_counts(results) estimate = combined_instruction_estimate(counts) print(f'{"*" * 80}\nCombined instruction estimate: {estimate:,}') # noqa: T201 + raise SystemExit(exit_code) if __name__ == '__main__': From d28001baa8bd0c65b0ead69a5c803fd7e1b7222e Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Sat, 30 May 2026 14:20:18 -0400 Subject: [PATCH 16/18] Fast-fail early native duplicate updates Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- bidict/_base.py | 34 +++++++++++++++++++--------------- tests/test_native.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/bidict/_base.py b/bidict/_base.py index 484d8e5f..124c25b1 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -62,6 +62,7 @@ _MIN_NATIVE_UPDATE_ITEMS = 8192 _MIN_NATIVE_FORCEUPDATE_ITEMS = 4096 _MIN_NATIVE_DUPVAL_PRESCAN_ITEMS = 16384 +_MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS = 64 def _native_items(arg: MapOrItems[KT, VT], kw: Mapping[str, VT]) -> Iterable[tuple[KT, VT]]: @@ -74,6 +75,19 @@ def _supports_native_mapping(arg: MapOrItems[KT, VT], kw: Mapping[str, VT]) -> b return not kw and isinstance(arg, Mapping) +def _prescan_mapping_dupvals(mapping: Mapping[KT, VT], max_items: int | None = None) -> None: + seen_by_val: dict[VT, KT] = {} + seen_get = seen_by_val.get + for index, (key, val) in enumerate(mapping.items()): + if max_items is not None and index >= max_items: + return + prev_key = seen_get(val, MISSING) + if prev_key is MISSING: + seen_by_val[val] = key + elif prev_key != key: + raise ValueDuplicationError(val) + + class BidictKeysView(KeysView[KT], ValuesView[KT]): """Since the keys of a bidict are the values of its inverse (and vice versa), the :class:`~collections.abc.ValuesView` result of calling *bi.values()* @@ -257,22 +271,10 @@ def _should_use_native_update(self, incoming_len: int | None, on_dup: OnDup) -> def _maybe_prescan_native_update( self, arg: MapOrItems[KT, VT], kw: Mapping[str, VT], on_dup: OnDup, incoming_len: int | None ) -> None: - if ( - kw - or incoming_len is None - or incoming_len < _MIN_NATIVE_DUPVAL_PRESCAN_ITEMS - or not isinstance(arg, Mapping) - or on_dup.val is not RAISE - ): + if kw or incoming_len is None or not isinstance(arg, Mapping) or on_dup.val is not RAISE: return - seen_by_val: dict[VT, KT] = {} - seen_get = seen_by_val.get - for key, val in arg.items(): - prev_key = seen_get(val, MISSING) - if prev_key is MISSING: - seen_by_val[val] = key - elif prev_key != key: - raise ValueDuplicationError(val) + max_items = None if incoming_len >= _MIN_NATIVE_DUPVAL_PRESCAN_ITEMS else _MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS + _prescan_mapping_dupvals(arg, max_items) @property def inv(self) -> BidictBase[VT, KT]: @@ -516,6 +518,8 @@ def _update( if not self and self._supports_native_map_swap(): if _supports_native_mapping(arg, kw) and _build_bidict_maps_from_mapping is not None: mapping_arg = t.cast(Mapping[t.Any, t.Any], arg) + if on_dup.val is RAISE: + _prescan_mapping_dupvals(mapping_arg, _MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS) self._set_map_data(*_build_bidict_maps_from_mapping(mapping_arg, on_dup)) return if _build_bidict_maps is not None: diff --git a/tests/test_native.py b/tests/test_native.py index 0139bf0e..f3537300 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -74,6 +74,21 @@ def fake_build_from_mapping(mapping: dict[int, int], _on_dup: object) -> tuple[d assert seen_mapping is mapping +def test_empty_mapping_update_prescans_early_dupvals_before_native_builder(monkeypatch: pytest.MonkeyPatch) -> None: + def fail_build_from_mapping(_mapping: object, _on_dup: object) -> tuple[dict[int, int], dict[int, int]]: + msg = 'native builder should not run after early duplicate-value prescan fails' + raise AssertionError(msg) + + monkeypatch.setattr(base_mod, '_build_bidict_maps_from_mapping', fail_build_from_mapping) + monkeypatch.setattr(base_mod, '_MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS', 2) + bi = bidict[int, int]() + + with pytest.raises(ValueDuplicationError): + bi.update({1: 0, 2: 0, 3: 3}) + + assert not bi + + def test_empty_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: items_seen: list[tuple[int, int]] = [] bi = bidict[int, int]() @@ -234,6 +249,25 @@ def fail_update( assert dict(bi.items()) == {10: 10} +def test_mapping_dupval_failure_prescans_early_before_native_update(monkeypatch: pytest.MonkeyPatch) -> None: + def fail_update( + _fwd: object, _inv: object, _items: object, _on_dup: object + ) -> tuple[dict[int, int], dict[int, int]]: + msg = 'native updater should not run after early duplicate-value prescan fails' + raise AssertionError(msg) + + monkeypatch.setattr(base_mod, '_update_bidict_maps_from_mapping', fail_update) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 1) + monkeypatch.setattr(base_mod, '_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS', 999) + monkeypatch.setattr(base_mod, '_MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS', 2) + bi = bidict({10: 10}) + + with pytest.raises(ValueDuplicationError): + bi.update({1: 0, 2: 0, 3: 3}) + + assert dict(bi.items()) == {10: 10} + + def test_nonempty_native_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: bi = bidict({1: 2, 3: 4}) inv = bi.inverse From c31f471f615348416948c44229c18dfc8f235d63 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Mon, 1 Jun 2026 10:47:32 -0400 Subject: [PATCH 17/18] Disable native helper on non-CPython Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- bidict/_native.py | 7 ++++++- tests/test_native.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/bidict/_native.py b/bidict/_native.py index 70c09b85..3fb9128b 100644 --- a/bidict/_native.py +++ b/bidict/_native.py @@ -9,6 +9,7 @@ from __future__ import annotations import os +import sys import typing as t from collections.abc import Iterable from collections.abc import Mapping @@ -42,6 +43,10 @@ def _native_disabled() -> bool: return value is not None and value.lower() in _DISABLE_NATIVE_TRUE_VALUES +def _native_supported_runtime() -> bool: + return sys.implementation.name == 'cpython' + + if t.TYPE_CHECKING: build_bidict_maps = None update_bidict_maps = None @@ -77,7 +82,7 @@ def _update_bidict_maps_from_mapping_impl( ) -> tuple[dict[t.Any, t.Any], dict[t.Any, t.Any]]: ... else: - if _native_disabled(): + if _native_disabled() or not _native_supported_runtime(): build_bidict_maps: BuildBidictMaps | None = None update_bidict_maps: UpdateBidictMaps | None = None build_bidict_maps_from_mapping: BuildBidictMapsFromMapping | None = None diff --git a/tests/test_native.py b/tests/test_native.py index f3537300..e0600be6 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -7,7 +7,9 @@ from __future__ import annotations import importlib +import sys from collections.abc import Iterable +from types import SimpleNamespace import pytest @@ -35,6 +37,22 @@ def test_native_env_var_disables_helpers(monkeypatch: pytest.MonkeyPatch) -> Non importlib.reload(reloaded) +def test_non_cpython_runtime_disables_helpers(monkeypatch: pytest.MonkeyPatch) -> None: + original_implementation = sys.implementation + implementation_attrs = vars(original_implementation).copy() + implementation_attrs['name'] = 'pypy' + monkeypatch.setattr(sys, 'implementation', SimpleNamespace(**implementation_attrs)) + reloaded = importlib.reload(native_mod) + try: + assert reloaded.build_bidict_maps is None + assert reloaded.build_bidict_maps_from_mapping is None + assert reloaded.update_bidict_maps is None + assert reloaded.update_bidict_maps_from_mapping is None + finally: + monkeypatch.setattr(sys, 'implementation', original_implementation) + importlib.reload(reloaded) + + def test_empty_update_uses_native_builder_when_available(monkeypatch: pytest.MonkeyPatch) -> None: items_seen: list[tuple[int, int]] = [] From 5f469f2ca698463e4a04877e90f6c0a918232153 Mon Sep 17 00:00:00 2001 From: Joshua Bronson Date: Sat, 6 Jun 2026 11:25:19 -0400 Subject: [PATCH 18/18] Fix native update duplication precedence Remove the non-empty native-update duplicate-value prescan so mapping updates preserve the same duplication-error precedence as the Python path, and update native tests to pin the intended routing and precedence behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- bidict/_base.py | 10 ---------- tests/test_native.py | 39 +++++++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/bidict/_base.py b/bidict/_base.py index 124c25b1..5c51ca27 100644 --- a/bidict/_base.py +++ b/bidict/_base.py @@ -61,7 +61,6 @@ ReversedIter: t.TypeAlias = t.Callable[['BidictBase[KT, t.Any]'], Iterator[KT]] _MIN_NATIVE_UPDATE_ITEMS = 8192 _MIN_NATIVE_FORCEUPDATE_ITEMS = 4096 -_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS = 16384 _MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS = 64 @@ -268,14 +267,6 @@ def _should_use_native_update(self, incoming_len: int | None, on_dup: OnDup) -> min_items = _MIN_NATIVE_FORCEUPDATE_ITEMS if on_dup.val is DROP_OLD else _MIN_NATIVE_UPDATE_ITEMS return incoming_len >= min(len(self), min_items) - def _maybe_prescan_native_update( - self, arg: MapOrItems[KT, VT], kw: Mapping[str, VT], on_dup: OnDup, incoming_len: int | None - ) -> None: - if kw or incoming_len is None or not isinstance(arg, Mapping) or on_dup.val is not RAISE: - return - max_items = None if incoming_len >= _MIN_NATIVE_DUPVAL_PRESCAN_ITEMS else _MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS - _prescan_mapping_dupvals(arg, max_items) - @property def inv(self) -> BidictBase[VT, KT]: """Alias for :attr:`inverse`.""" @@ -527,7 +518,6 @@ def _update( return if self._should_use_native_update(incoming_len, on_dup): - self._maybe_prescan_native_update(arg, kw, on_dup, incoming_len) fwdm = t.cast(dict[t.Any, t.Any], self._fwdm) invm = t.cast(dict[t.Any, t.Any], self._invm) if _supports_native_mapping(arg, kw) and _update_bidict_maps_from_mapping is not None: diff --git a/tests/test_native.py b/tests/test_native.py index e0600be6..d61264ce 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -15,7 +15,9 @@ from bidict import DROP_NEW from bidict import ON_DUP_DROP_OLD +from bidict import RAISE from bidict import KeyAndValueDuplicationError +from bidict import KeyDuplicationError from bidict import OnDup from bidict import OrderedBidict from bidict import ValueDuplicationError @@ -249,41 +251,46 @@ def fake_update( assert dict(bi.items()) == {1: 2, 5: 4, 6: 7} -def test_large_mapping_dupval_failure_prescans_before_native_update(monkeypatch: pytest.MonkeyPatch) -> None: +def test_nonempty_mapping_dupval_failure_uses_native_updater(monkeypatch: pytest.MonkeyPatch) -> None: + called = False + def fail_update( - _fwd: object, _inv: object, _items: object, _on_dup: object + _fwd: object, _inv: object, _mapping: object, _on_dup: object ) -> tuple[dict[int, int], dict[int, int]]: - msg = 'native updater should not run after duplicate-value prescan fails' - raise AssertionError(msg) + nonlocal called + called = True + raise ValueDuplicationError(0) - monkeypatch.setattr(base_mod, '_update_bidict_maps', fail_update) + monkeypatch.setattr(base_mod, '_update_bidict_maps_from_mapping', fail_update) monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 1) - monkeypatch.setattr(base_mod, '_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS', 2) bi = bidict({10: 10}) with pytest.raises(ValueDuplicationError): bi.update({1: 0, 2: 0}) + assert called assert dict(bi.items()) == {10: 10} -def test_mapping_dupval_failure_prescans_early_before_native_update(monkeypatch: pytest.MonkeyPatch) -> None: +def test_nonempty_mapping_update_preserves_duplication_precedence(monkeypatch: pytest.MonkeyPatch) -> None: + called = False + def fail_update( - _fwd: object, _inv: object, _items: object, _on_dup: object + _fwd: object, _inv: object, _mapping: object, _on_dup: object ) -> tuple[dict[int, int], dict[int, int]]: - msg = 'native updater should not run after early duplicate-value prescan fails' - raise AssertionError(msg) + nonlocal called + called = True + raise KeyDuplicationError(1) monkeypatch.setattr(base_mod, '_update_bidict_maps_from_mapping', fail_update) monkeypatch.setattr(base_mod, '_MIN_NATIVE_UPDATE_ITEMS', 1) - monkeypatch.setattr(base_mod, '_MIN_NATIVE_DUPVAL_PRESCAN_ITEMS', 999) - monkeypatch.setattr(base_mod, '_MAX_NATIVE_DUPVAL_FAST_FAIL_ITEMS', 2) - bi = bidict({10: 10}) + bi = bidict({1: 10}) - with pytest.raises(ValueDuplicationError): - bi.update({1: 0, 2: 0, 3: 3}) + with pytest.raises(KeyDuplicationError): + bi.putall({1: 0, 2: 0, 3: 3}, OnDup(RAISE, RAISE)) - assert dict(bi.items()) == {10: 10} + assert called + assert dict(bi.items()) == {1: 10} def test_nonempty_native_update_preserves_materialized_inverse(monkeypatch: pytest.MonkeyPatch) -> None: