diff --git a/news/524.bugfix.md b/news/524.bugfix.md new file mode 100644 index 00000000..5d2277da --- /dev/null +++ b/news/524.bugfix.md @@ -0,0 +1,4 @@ +Charts over several disk-backed columns from one file (the rows of a 2-D +`np.memmap` table, or `np.memmap(offset=...)` columns packed into one file) no +longer reuse one column's cached statistics for another, which gave a column +the other column's axis range when the chart was built again. diff --git a/python/xy/_ooc.py b/python/xy/_ooc.py index 68c89721..a97f3e62 100644 --- a/python/xy/_ooc.py +++ b/python/xy/_ooc.py @@ -133,6 +133,31 @@ def backing_path(arr: Any) -> str | None: return None +def backing_offset(arr: Any) -> int | None: + """Byte offset of ``arr``'s first element within its backing file, or + ``None`` when ``arr`` is not disk-backed. + + Views of one file share its path, so the path alone does not name a + column: the rows of a 2-D table memmap, or two ``np.memmap(offset=...)`` + columns packed into one file, all map the same file. A view's + ``.offset`` is inherited from the memmap it was sliced from, so the + offset is measured from the root memmap (the one mapping the file + directly, whose ``.offset`` is its file position) to ``arr``'s data. + """ + root = None + seen = arr + while isinstance(seen, np.ndarray): + if isinstance(seen, np.memmap): + root = seen + seen = seen.base + root_offset = getattr(root, "offset", None) + if root is None or root_offset is None: + return None + start = int(root.__array_interface__["data"][0]) + here = int(np.asarray(arr).__array_interface__["data"][0]) + return int(root_offset) + (here - start) + + def open_f64(path: str | os.PathLike[str]) -> npt.NDArray[np.float64]: """Reopen an existing canonical f64 file as a read-only memmap column. diff --git a/python/xy/columns.py b/python/xy/columns.py index 7d37a745..dfd6b01c 100644 --- a/python/xy/columns.py +++ b/python/xy/columns.py @@ -98,7 +98,10 @@ def null_count(self) -> int: # restart) loads a ~10 MB sidecar in well under a second. The cache self- # invalidates if the source file's size or mtime changed, or the chunk size / # row count no longer matches, so a stale/edited column is never trusted. -_ZONE_CACHE_MAGIC = b"XYZONEC1" +# Version 2 names the sidecar by the column's byte offset in its file (see +# `_zone_cache_path`). A version-1 sidecar may hold another column's maps, so +# the magic changed with the name and old files are never read. +_ZONE_CACHE_MAGIC = b"XYZONEC2" _ZONE_CACHE_SUFFIX = ".xyzones" # 6 f64 planes + 2 u64 planes per chunk (mirrors ZoneMaps' fields, in order). _ZONE_F64_FIELDS = ("mins", "maxs", "sums", "sum_sqs", "positive_mins", "positive_maxs") @@ -106,8 +109,22 @@ def null_count(self) -> int: def _zone_cache_path(arr: Any) -> str | None: + """Sidecar path for one memmapped column. + + Several columns can map the same file (rows of a 2-D table memmap, or + `np.memmap(offset=...)` columns packed into one file), so the sidecar is + named `..xyzones`: sharing one path let a same-length + column reload another column's maps (its min/max, hence its axis range). + The offset is always written, even when it is 0, so `table` at offset 8 + and a file named `table.8` at offset 0 cannot share a name either. + """ bp = _ooc.backing_path(arr) - return (bp + _ZONE_CACHE_SUFFIX) if bp is not None else None + if bp is None: + return None + offset = _ooc.backing_offset(arr) + if offset is None: + return None + return f"{bp}.{offset}{_ZONE_CACHE_SUFFIX}" def _zone_source_stamp(source: str) -> tuple[int, int]: diff --git a/spec/process/perf-audit-2026-07-22.md b/spec/process/perf-audit-2026-07-22.md index 78f09941..2da12a43 100644 --- a/spec/process/perf-audit-2026-07-22.md +++ b/spec/process/perf-audit-2026-07-22.md @@ -35,12 +35,16 @@ columns) that is a ~53 s NVMe-bound scan paid on **every** process start an immutable file. Fix (`columns.py`, Python side — no kernel/ABI change): persist the fold to a -`.xyzones` sidecar and reload it on the next build instead of +sidecar file and reload it on the next build instead of rescanning. The sidecar carries the source file's size + mtime and the `(ZONE_CHUNK, row-count)` it was folded at; any mismatch rejects it, so an -edited or truncated column is never trusted. First build pays the scan and -writes the sidecar (21 MB for both planet columns); every later build loads it -bit-identically. Measured on the real 10.74 B-row planet columns: **52.8 s → +edited or truncated column is never trusted. The sidecar is named +`..xyzones` because several columns can map one file (the +rows of a 2-D table memmap, or `np.memmap(offset=...)` columns); sharing a +name lets a same-length column reload another column's maps. First build pays +the scan and writes the sidecar (21 MB for both planet columns); every later +build loads it bit-identically. Measured on the real 10.74 B-row planet +columns: **52.8 s → 0.01 s** figure build, identical domain. The fused `zone_maps_pair` still does both columns in one pass on a cold miss; only genuinely disk-backed columns are cached (in-RAM ingest is byte-for-byte unchanged). Full range-pruning is diff --git a/tests/test_ooc.py b/tests/test_ooc.py index 59fa22c9..0060978b 100644 --- a/tests/test_ooc.py +++ b/tests/test_ooc.py @@ -168,3 +168,69 @@ def test_zone_map_cache_roundtrip_and_staleness(tmp_path): os.utime(tmp_path / "z.f64", ns=(0, 0)) reopened = open_f64(tmp_path / "z.f64") assert _load_zone_cache(reopened) is None + + +def test_zone_map_cache_keeps_views_of_one_file_apart(tmp_path): + """Two same-length columns mapped from one file (rows of a 2-D table memmap) + each keep their own zone-map sidecar, so a warm rebuild does not hand one + column the other's min/max.""" + n = 1000 + path = tmp_path / "table.f64" + table = np.memmap(path, dtype=np.float64, mode="w+", shape=(2, n)) + table[0] = np.linspace(0.0, 1.0, n) + table[1] = np.linspace(1000.0, 2000.0, n) + table.flush() + del table + + def build_ranges(): + mapped = np.memmap(path, dtype=np.float64, mode="r", shape=(2, n)) + fig = xy.chart(xy.scatter(x=mapped[0], y=mapped[1])).figure() + trace = fig.traces[0] + return (trace.x.min, trace.x.max), (trace.y.min, trace.y.max) + + from xy.columns import _load_zone_cache + + cold = build_ranges() # folds both columns and writes the sidecars + mapped = np.memmap(path, dtype=np.float64, mode="r", shape=(2, n)) + assert _load_zone_cache(mapped[0]).max == 1.0 # each row persisted its own + assert _load_zone_cache(mapped[1]).min == 1000.0 + warm = build_ranges() # reloads them + assert cold == ((0.0, 1.0), (1000.0, 2000.0)) + assert warm == cold + + +def test_zone_map_cache_keys_on_file_offset(tmp_path): + """`np.memmap(..., offset=)` columns of one file cache separately too.""" + from xy import columns + + n = 500 + path = tmp_path / "packed.f64" + np.concatenate([np.full(n, -5.0), np.full(n, 7.0)]).tofile(path) + + def view(offset): + return np.memmap(path, dtype=np.float64, mode="r", offset=offset, shape=(n,)) + + assert columns._zone_maps_for(view(0)).max == -5.0 + assert columns._zone_maps_for(view(n * 8)).min == 7.0 + # Warm reloads read each column's own sidecar. + assert columns._load_zone_cache(view(0)).max == -5.0 + assert columns._load_zone_cache(view(n * 8)).min == 7.0 + assert columns._zone_cache_path(view(0)) != columns._zone_cache_path(view(n * 8)) + + # A file whose name ends in `.` gets a distinct sidecar from the + # same-named column at that offset of the shorter-named file. + suffixed = tmp_path / f"packed.f64.{n * 8}" + np.full(n, 3.0).tofile(suffixed) + at_zero = np.memmap(suffixed, dtype=np.float64, mode="r", shape=(n,)) + assert columns._zone_cache_path(at_zero) != columns._zone_cache_path(view(n * 8)) + assert columns._zone_maps_for(at_zero).max == 3.0 + assert columns._load_zone_cache(view(n * 8)).min == 7.0 + + +def test_backing_offset_without_file_metadata_is_not_disk_backed(): + """A memmap-typed view with no mapping behind it has no file offset.""" + from xy._ooc import backing_offset + + orphan = np.arange(4, dtype=np.float64).view(np.memmap) + assert backing_offset(orphan) is None + assert backing_offset(np.arange(4, dtype=np.float64)) is None