Cache dataset & exchange-set metadata via cross-session sidecars (#467)#468
Merged
Conversation
Expose a memoized, product-agnostic DatasetMetadata (spec, extent, CRS, display scale, time coverage) derived once from the dataset each processor already parsed — never a second parse or a static ReadMetadata(path) call. - IDatasetProcessor gains a default Metadata member (Spec-only) so non-overriding implementers still compile. - GmlDatasetProcessorBase memoizes the raw WGS-84 envelope and shares that single feature scan between the padded render extent (ComputeGeographicExtent) and Metadata.Extent, removing the per-render O(n) coordinate rescan (previously run on every render). - S101 derives Metadata from the already-parsed dataset; S102/S104/S111 derive extent + CRS from the coverage source's already-read georeferencing metadata (no HDF5 payload re-read). S104/S111 dcf8 station series union their station coordinates. - Tests: GML raw extent agrees with the padded render extent and is memoized; S102 UTM cell surfaces CRS + extent; S104/S111 dcf8 derive the station bounding box. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
Consolidate the MCP catalog's GML and S-101 projection helpers onto the canonical dataset ReadMetadata() surface, completing issue #467 WS1's metadata-as-byproduct goal for the viewer. - ProjectGml now returns (data, dataset.ReadMetadata()); the declared product edition and raw WGS-84 envelope come from GmlDatasetMetadata, replacing the hand-rolled ComputeGmlBounds walk. The catalog keeps its canonical spec name (S-57 -> S-101 mapping stays upstream in TryProject) and only adopts the metadata edition. - ProjectS101 derives Spec + Extent from S101Dataset.ReadMetadata(), replacing ResolveS101Edition + ComputeS101Bounds. - Delete ResolveS101Edition, ComputeS101Bounds, ComputeGmlBounds and the now-unused EncDotNet.S100.Features using. - GML entries now surface the declared edition (was default) — an intentional improvement; no test pinned GML edition to default. Tests: retarget the four helper tests to the canonical ReadMetadata API and add a GML bounds parity test. All 15 ViewerDatasetCatalog tests pass; formatters (whitespace + IDE0005) clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
Introduce the foundational, format-agnostic per-dataset metadata sidecar that makes the relaxed parse-once rule pay off (issue #467 WS3): a persisted DatasetMetadata lets a later session recover a dataset's cheap "peek" facts (spec, extent, CRS, display-scale window, temporal coverage) without re-parsing, and is the primary enabler of loose-dataset lazy registration. - DatasetMetadataSerializer: versioned binary serialize / total TryDeserialize of DatasetMetadata; corruption or FormatVersion mismatch yields null (a miss), mirroring DrawingInstructionSerializer. - IDatasetMetadataCache + DiskDatasetMetadataCache: one .dmeta sidecar per dataset keyed by source last-write time + length; a stale, unwritable, or corrupt entry is a miss that never breaks loading. Atomic temp+move writes, LRU byte cap, thread-safe (single lock) — same robustness contract as DiskPortrayalInstructionCache. GetOrRead runs the cold producer on a miss and persists; TryGet is a pure probe. Tests: 14 new (7 serializer round-trip / corruption / version, 7 disk cache hit-avoids-parse, cross-instance persistence, mtime/size invalidation, corrupt=miss, missing-source pass-through). Core README updated; formatters clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
Wire the cross-session per-dataset metadata sidecar (Core WS3 Slice 1) into ExchangeSetService.OpenLooseCellFolderAsync. Each S-101 base cell's extent is read once through IDatasetMetadataReader / CachingDatasetMetadataReader (backed by DiskDatasetMetadataCache, keyed by source mtime+size) and unioned via ComputeLooseCellUnion, so a loose `.000` folder frames its extent up front and reuses cached extents on later sessions with no re-parse. S-57 cells have no cheap extent reader and are skipped from the union (graceful degradation preserving the prior null-union behaviour when no extent is available). - Add IDatasetMetadataReader + CachingDatasetMetadataReader (S-101 wired via S101Dataset.ReadMetadata; unknown/unsupported product => null, no negative caching). - Register IDatasetMetadataCache (DiskDatasetMetadataCache, 16 MB cap) and IDatasetMetadataReader singletons; add ViewerDataPaths.DatasetMetadataCacheDirectory to CacheDirectories. - Thread onFramingReady + UnionBoundingBox through the loose path. - Tests: ExchangeSetServiceLooseCellUnionTests (union/skip/null) and CachingDatasetMetadataReaderTests (empty/unknown => null; S-101 read then hit). READMEs updated. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
…ice 2) Re-opening a large S-57 / S-63 exchange set re-parsed its binary CATALOG.031 every time. Add a cross-session sidecar that caches the base-cell descriptor list (cell name, base-cell relative path, ordered update relative paths, EPSG:4326 bbox) keyed by the catalogue file's mtime + size, so a re-open replays descriptors straight into lazy registration with zero ISO 8211 re-parse. Dataset bytes are still read on demand, so the cache never serves stale chart content. - Add S57CatalogCacheSerializer (versioned binary, corruption=miss), IS57CatalogCache + DiskS57CatalogCache (atomic write, mtime+size key, LRU byte cap, miss-never-breaks) under Viewer Services/Caching/, mirroring DiskDatasetMetadataCache. - ExchangeSetService.ReadBaseCellsCached wraps S57ExchangeSetCatalog.ReadBaseCells for both the eager and lazy S-57 open paths; ResolveCataloguePath keys the entry and preserves the existing catalogue-not-found handling. Null cache => direct read. - Register IS57CatalogCache (16 MB cap) via DI; add ViewerDataPaths.S57CatalogCacheDirectory to CacheDirectories. - Tests: serializer round-trip/corruption (4), disk cache hit/invalidate/corrupt/missing (5), and an end-to-end re-open hit against the synthetic Synthetic-S57-Framed CATALOG.031 fixture (1). README updated. Scoped to S-57: only the S-57 branch has lazy load and consumes purely catalogue-derived descriptors. The S-100 and S-57-eager paths still need the live catalogue object to dispatch datasets, so caching there would only shift framing marginally. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
Contributor
Performance Gate✅ PASSED — no regressions. Threshold: 10.0%, MAD multiplier (k): 3.0, retry-zone mult: 2.0× Scenario summary
exchange-set-openIteration statistics
Spans (sum of all iterations)
Metrics
s101-portray-coldIteration statistics
s101-portray-warmIteration statistics
s101-real-coldIteration statistics
s101-real-warmIteration statistics
s101-render-warmIteration statistics
s102-coverageIteration statistics
s102-coverage-openIteration statistics
Spans (sum of all iterations)
Metrics
s102-coverage-render-largeIteration statistics
s102-real-warmIteration statistics
s111-real-warmIteration statistics
s124-vectorIteration statistics
s201-vectorIteration statistics
Generated by EncDotNet.S100.PerfReport gate command |
There was a problem hiding this comment.
Pull request overview
This PR implements issue #467’s metadata-caching plan to speed up dataset loading and exchange-set re-open by (1) making DatasetMetadata a byproduct of existing dataset parses and (2) persisting “peek” metadata and S-57 catalogue descriptors as cross-session on-disk sidecars for re-open hits.
Changes:
- Added
IDatasetProcessor.Metadataand implemented memoized, parse-byproduct metadata across S-101/S-102/S-104/S-111 processors andGmlDatasetProcessorBase(single feature scan shared with render extent computation). - Introduced disk sidecar caches:
DatasetMetadataSerializer+DiskDatasetMetadataCache(Core) andS57CatalogCacheSerializer+DiskS57CatalogCache(Viewer). - Updated Viewer services to use canonical
ReadMetadata()for bounds, added loose ENC cell union framing via cached metadata probes, and wired new caches via DI + ViewerDataPaths.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/EncDotNet.S100.VisualRegression.Tests/DatasetProcessorMetadataTests.cs | Adds regression test validating IDatasetProcessor.Metadata memoization + extent parity for a GML processor. |
| tests/EncDotNet.S100.Viewer.Tests/ViewerDatasetCatalogTests.cs | Updates tests to use ReadMetadata() for edition/bounds; adds parity guard for GML bounds. |
| tests/EncDotNet.S100.Viewer.Tests/ViewerDataPathsTests.cs | Updates expected cache-directory counts after new cache dirs were added. |
| tests/EncDotNet.S100.Viewer.Tests/S57CatalogCacheSerializerTests.cs | Adds serializer round-trip + corruption/version mismatch tests for S-57 catalogue sidecar payload. |
| tests/EncDotNet.S100.Viewer.Tests/ExchangeSetServiceLooseCellUnionTests.cs | Adds unit tests for loose-cell union framing based on metadata probes. |
| tests/EncDotNet.S100.Viewer.Tests/ExchangeSetServiceLoaderTests.cs | Adds E2E test ensuring S-57 catalogue descriptors are served from cache on reopen. |
| tests/EncDotNet.S100.Viewer.Tests/DiskS57CatalogCacheTests.cs | Adds disk cache behavior tests (hit/miss, invalidation, corrupt sidecar, missing source). |
| tests/EncDotNet.S100.Viewer.Tests/CachingDatasetMetadataReaderTests.cs | Adds tests for path-based metadata reader backed by disk cache (incl. real-data skip). |
| tests/EncDotNet.S100.Pipelines.Tests/S111Dcf8ProcessorTests.cs | Adds test verifying S-111 DCF8 processor metadata extent/memoization. |
| tests/EncDotNet.S100.Pipelines.Tests/S104Dcf8ProcessorTests.cs | Adds test verifying S-104 DCF8 processor metadata extent/memoization. |
| tests/EncDotNet.S100.Pipelines.Tests/S102Edition21Tests.cs | Adds test verifying S-102 processor metadata surfaces CRS/extent and is memoized. |
| tests/EncDotNet.S100.Core.Tests/DiskDatasetMetadataCacheTests.cs | Adds tests for disk-backed dataset metadata cache (hit/miss, invalidation, corruption). |
| tests/EncDotNet.S100.Core.Tests/DatasetMetadataSerializerTests.cs | Adds serializer round-trip + corruption/version mismatch tests for DatasetMetadata. |
| src/EncDotNet.S100.Viewer/ViewerDataPaths.cs | Adds new cache directories for dataset-metadata sidecars and S-57 catalogue descriptors. |
| src/EncDotNet.S100.Viewer/Services/ViewerDatasetCatalog.cs | Switches GML/S-101 bounds+edition to canonical ReadMetadata() and removes hand-rolled scans. |
| src/EncDotNet.S100.Viewer/Services/IDatasetMetadataReader.cs | Introduces internal interface for cheap per-file metadata probing. |
| src/EncDotNet.S100.Viewer/Services/ExchangeSetService.cs | Adds loose-cell union framing via metadata reader; adds cached S-57 base-cell reads via sidecar. |
| src/EncDotNet.S100.Viewer/Services/CachingDatasetMetadataReader.cs | Implements cached path-based metadata reader (currently wired for S-101). |
| src/EncDotNet.S100.Viewer/Services/Caching/S57CatalogCacheSerializer.cs | Implements binary serializer for cached S-57 base-cell descriptors. |
| src/EncDotNet.S100.Viewer/Services/Caching/IS57CatalogCache.cs | Adds abstraction for cross-session S-57 catalogue-descriptor cache. |
| src/EncDotNet.S100.Viewer/Services/Caching/DiskS57CatalogCache.cs | Implements disk-backed S-57 catalogue descriptor cache with atomic writes + LRU eviction. |
| src/EncDotNet.S100.Viewer/README.md | Documents loose-cell framing and S-57 catalogue descriptor caching behavior. |
| src/EncDotNet.S100.Viewer/App.axaml.cs | Wires dataset-metadata and S-57 catalogue caches + reader into DI container. |
| src/EncDotNet.S100.Datasets.Pipelines/S111DatasetProcessor.cs | Adds memoized Metadata derived from gridded source or fixed-station coordinates. |
| src/EncDotNet.S100.Datasets.Pipelines/S104DatasetProcessor.cs | Adds memoized Metadata derived from gridded source or fixed-station coordinates. |
| src/EncDotNet.S100.Datasets.Pipelines/S102DatasetProcessor.cs | Adds memoized Metadata derived from coverage georeferencing + CRS. |
| src/EncDotNet.S100.Datasets.Pipelines/S101DatasetProcessor.cs | Adds memoized Metadata derived from already-parsed S-101 dataset. |
| src/EncDotNet.S100.Datasets.Pipelines/README.md | Documents “metadata as a parse byproduct” behavior and expectations. |
| src/EncDotNet.S100.Datasets.Pipelines/IDatasetProcessor.cs | Adds Metadata to processor interface via default implementation. |
| src/EncDotNet.S100.Datasets.Pipelines/GmlDatasetProcessorBase.cs | Memoizes a raw envelope once and reuses it for metadata extent + padded render extent. |
| src/EncDotNet.S100.Core/README.md | Updates Core README to document the new cross-session metadata sidecar cache. |
| src/EncDotNet.S100.Core/Metadata/IDatasetMetadataCache.cs | Adds public cache interface for persisted DatasetMetadata keyed by mtime/size. |
| src/EncDotNet.S100.Core/Metadata/DiskDatasetMetadataCache.cs | Implements disk-backed metadata cache with atomic writes + LRU eviction + robustness contract. |
| src/EncDotNet.S100.Core/Metadata/DatasetMetadataSerializer.cs | Implements versioned binary serializer for DatasetMetadata used by the disk cache. |
The XML doc said the value is "derived once ... and cached", but the default interface implementation returns a fresh Spec-only instance on each access. Distinguish the overriding derive-once-and-cache behaviour from the default per-access minimal value. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
TryGet increments the miss counter without invoking a producer, so the "fell through to the producer" wording was inaccurate for TryGet misses. Document Misses as any lookup not served from a valid cached entry, via either GetOrRead (where the producer runs) or TryGet (returns false). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
- GmlDatasetProcessorBase: make the memoized RawEnvelope a reference type so the unsynchronized ??= publishes it via an atomic reference write. Metadata (catalog framing) and ComputeGeographicExtent (render) can run on different threads; a multi-field record struct risked torn reads. - S57CatalogCacheSerializer.TryDeserialize: build the update paths as a List<string> directly and upcast once, dropping the brittle IReadOnlyList->List cast in the fill loop. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
- DiskDatasetMetadataCache / DiskS57CatalogCache: reject a payload length larger than the remaining sidecar bytes before ReadBytes allocates, so a corrupt frame degrades to a cache miss instead of a huge allocation/OOM. - ExchangeSetService.OpenLooseCellFolderAsync: run the up-front loose-cell union probe on a worker thread (Task.Run). OpenAsync is dispatched on the UI thread, and a first-open cache miss parses every cell's metadata; that work must not block the UI. First-open framing is preserved (the probe is the intentional, cross-session-cached parse under the relaxed rule). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
BinaryReader.ReadString() allocates from a 7-bit length prefix before it discovers EOF, so a corrupt sidecar could trigger a very large allocation instead of degrading to a cache miss. Route every string read in DatasetMetadataSerializer and S57CatalogCacheSerializer through a ReadBoundedString helper that rejects a length prefix larger than the remaining (in-memory, size-framed) payload before allocating. The read stays wire-compatible with BinaryWriter.Write(string). Adds a regression test per serializer for an oversized length prefix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a68ee5d9-dc24-43b6-b6dc-1d0d843f0eb8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the metadata-caching work tracked in #467 to speed up dataset loading and exchange-set re-open by (a) making metadata a byproduct of the parse we already do, and (b) persisting it in cross-session disk sidecars so a re-open skips re-parsing.
What landed
WS1 — metadata as a parse byproduct
IDatasetProcessor.Metadataso every processor exposes itsDatasetMetadata(extent, name, edition, …) as a byproduct of the load it already performs — no separate metadata read.GmlDatasetProcessorBase.ViewerDatasetCatalogthrough the canonicalReadMetadatapath, removing the per-render extent rescan.WS3 — cross-session metadata sidecars
DatasetMetadataSerializer+DiskDatasetMetadataCache(EncDotNet.S100.Core/Metadata/) — an mtime+size-keyed, atomically-written, LRU-capped disk sidecar forDatasetMetadata.IDatasetMetadataReader/CachingDatasetMetadataReaderprobe each S-101 base cell's extent (cheap ISO 8211 metadata peek) so a folder of loose ENC cells is framed to its union bounding box up front. S-57 cells are skipped (no cheap extent).S57CatalogCacheSerializer+IS57CatalogCache/DiskS57CatalogCache(ViewerServices/Caching/) cache the S-57CATALOG.031base-cell descriptors so lazy registration of a large exchange set skips re-parsing the catalogue on re-open.App.axaml.cs; new cache directories inViewerDataPaths(DatasetMetadataCacheDirectory,S57CatalogCacheDirectory).Closed without changes
WS4 — smaller memoizations: investigated and closed.
GetPortrayalContentHashAsyncis already memoized and the dataset-bytes hash is computed once in the constructor (nothing to eliminate); compiled-Lua caching is infeasible/low-value (no compiled-form hook inILuaContext; the instruction cache already skips the whole Lua run on a hit).WS2 — peek→promote object reuse: remains optional/deferred; only worth doing if profiling shows the now-permitted in-session double parse hurts a real eager flow.
Tests
New xunit coverage for every new public API: metadata serializer round-trip + disk cache, per-processor
Metadatabyproduct, loose-cell union framing, the S-57 catalogue serializer + disk cache, and an end-to-end re-open-hit against theSynthetic-S57-Framedfixture. Real-data tests useSkip.If. FullEncDotNet.S100.Viewer.Testssuite green.Closes #467