Skip to content

Commit f64a501

Browse files
beetlebugorgclaude
andcommitted
perf(s57): short-circuit header reads; stop full-parsing cells for bounds
S-57 headers carry no bounding box: DSID gives identity, DSPM gives the compilation scale (→ band), but a cell's extent comes only from M_COVR coverage features or the exchange-set CATALOG.031. Several paths were doing far more work than that to recover metadata. - iso8211.Parser.Next(): caller-driven, one-record-at-a-time reader so a caller can stop after the leading records instead of slurping the file. - s57.ReadHeader/ReadHeaderFS → CellHeader: reads only DSID/DSPM (no features, no geometry), reusing parseDSID/parseDSPM. Cross-checked field-for-field against a full Parse. - cell index: was a full ParseCellBytes (geometry + R-tree) per cell just for a bbox; now an M_COVR-only coverage parse, with a full-parse fallback for the rare cell lacking M_COVR. Fixed the docstring that wrongly claimed it read "each cell's header once". - ExtractCellMeta: now takes the *s57.Catalog. When the catalogue already supplies a cell's bbox AND the cell has no updates (so the base-cell header identity is current), read just the header and take the catalogue bbox, skipping the M_COVR parse. Otherwise unchanged update-applied parse — the no-updates guard preserves correct UPDN/ISDT/edition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 266f3ef commit f64a501

9 files changed

Lines changed: 383 additions & 39 deletions

File tree

internal/engine/baker/meta.go

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package baker
22

33
import (
4+
"path"
45
"sort"
56
"strconv"
7+
8+
"github.com/beetlebugorg/chartplotter/pkg/iso8211"
9+
"github.com/beetlebugorg/chartplotter/pkg/s57"
610
)
711

812
// CellMeta is the per-cell metadata extracted at import time for the chart
@@ -23,49 +27,101 @@ type CellMeta struct {
2327
HasBBox bool `json:"-"`
2428
}
2529

26-
// ExtractCellMeta parses each cell's header + coverage (coverage-only, cheap) and
27-
// returns per-cell metadata keyed by cell stem. Cells that fail to parse are
28-
// reported via onSkip and omitted. Title is left empty (S-57 headers carry no human
29-
// chart name — only the cell code); the caller overlays the CATALOG.031 long name
30-
// where the exchange set provides one.
31-
func ExtractCellMeta(cells map[string]CellData, onSkip func(name string, err error)) map[string]CellMeta {
30+
// ExtractCellMeta returns per-cell metadata keyed by cell stem. Identity and scale
31+
// come from each cell's S-57 header (DSID/DSPM); coverage comes from the exchange
32+
// -set catalogue when it covers the cell — sparing a parse — and otherwise from an
33+
// M_COVR-only coverage parse. Pass cat=nil when there is no catalogue.
34+
//
35+
// Cells that fail to parse are reported via onSkip and omitted. Title is left empty
36+
// (S-57 headers carry no human chart name — only the cell code); the caller overlays
37+
// the CATALOG.031 long name where the exchange set provides one.
38+
func ExtractCellMeta(cells map[string]CellData, cat *s57.Catalog, onSkip func(name string, err error)) map[string]CellMeta {
39+
catBBox := catalogBBoxes(cat)
3240
out := make(map[string]CellMeta, len(cells))
3341
names := make([]string, 0, len(cells))
3442
for n := range cells {
3543
names = append(names, n)
3644
}
3745
sort.Strings(names)
3846
for _, name := range names {
39-
cd := cells[name]
40-
chart, err := ParseCellCoverage(name, cd.Base, cd.Updates)
47+
m, err := cellMetaFor(name, cells[name], catBBox)
4148
if err != nil {
4249
if onSkip != nil {
4350
onSkip(name, err)
4451
}
4552
continue
4653
}
47-
stem := cellStem(chart.DatasetName())
48-
if stem == "" {
49-
stem = cellStem(name)
50-
}
51-
m := CellMeta{
52-
Name: stem,
53-
Scale: int(chart.CompilationScale()),
54-
Edition: chart.Edition(),
55-
Update: chart.UpdateNumber(),
56-
IssueDate: chart.IssueDate(),
57-
Agency: chart.ProducingAgency(),
58-
}
59-
b := chart.Bounds()
60-
if b.MaxLon > b.MinLon && b.MaxLat > b.MinLat {
61-
m.BBox = [4]float64{b.MinLon, b.MinLat, b.MaxLon, b.MaxLat}
62-
m.HasBBox = true
54+
out[m.Name] = m
55+
}
56+
return out
57+
}
58+
59+
// catalogBBoxes indexes an exchange-set catalogue's per-cell coverage by cell stem,
60+
// or returns nil when there's no catalogue / no coverage in it.
61+
func catalogBBoxes(cat *s57.Catalog) map[string][4]float64 {
62+
if cat == nil {
63+
return nil
64+
}
65+
out := map[string][4]float64{}
66+
for _, e := range cat.Cells() {
67+
if e.HasBBox {
68+
out[e.CellStem()] = [4]float64{e.West, e.South, e.East, e.North}
6369
}
64-
out[stem] = m
6570
}
6671
return out
6772
}
6873

74+
// cellMetaFor builds one cell's metadata. When the catalogue already supplies the
75+
// cell's coverage AND the cell has no updates (so its base-cell header still carries
76+
// the current identity), it reads only the header — DSID/DSPM, no geometry — and
77+
// takes the bbox from the catalogue, skipping the M_COVR coverage parse entirely.
78+
// Otherwise it falls back to the coverage parse, which also applies updates so the
79+
// reported edition/update/date reflect the cell's current state.
80+
func cellMetaFor(name string, cd CellData, catBBox map[string][4]float64) (CellMeta, error) {
81+
stem := cellStem(name)
82+
if len(cd.Updates) == 0 {
83+
if box, ok := catBBox[stem]; ok {
84+
p := "/" + path.Base(name)
85+
if h, err := s57.ReadHeaderFS(iso8211.MemFS{p: cd.Base}, p); err == nil {
86+
return CellMeta{
87+
Name: stem,
88+
Scale: int(h.CompilationScale),
89+
Edition: h.Edition,
90+
Update: h.UpdateNumber,
91+
IssueDate: h.IssueDate,
92+
Agency: h.ProducingAgency,
93+
BBox: box,
94+
HasBBox: true,
95+
}, nil
96+
}
97+
// Header read failed (malformed front matter) — fall through to a full parse.
98+
}
99+
}
100+
101+
chart, err := ParseCellCoverage(name, cd.Base, cd.Updates)
102+
if err != nil {
103+
return CellMeta{}, err
104+
}
105+
s := cellStem(chart.DatasetName())
106+
if s == "" {
107+
s = stem
108+
}
109+
m := CellMeta{
110+
Name: s,
111+
Scale: int(chart.CompilationScale()),
112+
Edition: chart.Edition(),
113+
Update: chart.UpdateNumber(),
114+
IssueDate: chart.IssueDate(),
115+
Agency: chart.ProducingAgency(),
116+
}
117+
b := chart.Bounds()
118+
if b.MaxLon > b.MinLon && b.MaxLat > b.MinLat {
119+
m.BBox = [4]float64{b.MinLon, b.MinLat, b.MaxLon, b.MaxLat}
120+
m.HasBBox = true
121+
}
122+
return m, nil
123+
}
124+
69125
// cellStem trims a trailing ".000"/".NNN" or directory path from a cell name.
70126
func cellStem(name string) string {
71127
// Strip any directory.

internal/engine/baker/meta_test.go

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@ package baker
33
import (
44
"os"
55
"testing"
6+
7+
"github.com/beetlebugorg/chartplotter/pkg/s57"
68
)
79

810
func TestExtractCellMeta(t *testing.T) {
911
data, err := os.ReadFile("../../../testdata/US5MD1MC.000")
1012
if err != nil {
1113
t.Fatal(err)
1214
}
13-
meta := ExtractCellMeta(map[string]CellData{"US5MD1MC.000": {Base: data}}, nil)
15+
meta := ExtractCellMeta(map[string]CellData{"US5MD1MC.000": {Base: data}}, nil, nil)
1416
m, ok := meta["US5MD1MC"]
1517
if !ok {
1618
t.Fatalf("no metadata for US5MD1MC; got keys %v", keys(meta))
@@ -33,6 +35,47 @@ func TestExtractCellMeta(t *testing.T) {
3335
}
3436
}
3537

38+
// TestExtractCellMeta_CatalogFastPath proves the catalogue short-circuit: when the
39+
// exchange-set catalogue already carries a (base) cell's coverage, identity is read
40+
// from the cheap header and the bbox is taken verbatim from the catalogue — no
41+
// M_COVR coverage parse. The stored bbox being the catalogue's exact rectangle (not
42+
// the geometry-derived M_COVR extent) is what confirms the fast path engaged.
43+
func TestExtractCellMeta_CatalogFastPath(t *testing.T) {
44+
data, err := os.ReadFile("../../../testdata/US5MD1MC.000")
45+
if err != nil {
46+
t.Fatal(err)
47+
}
48+
catData, err := os.ReadFile("../../../pkg/s57/testdata/US5MD1MC_CATALOG.031")
49+
if err != nil {
50+
t.Fatal(err)
51+
}
52+
cat, err := s57.ParseCatalog(catData)
53+
if err != nil {
54+
t.Fatal(err)
55+
}
56+
var catBox [4]float64
57+
for _, e := range cat.Cells() {
58+
if e.CellStem() == "US5MD1MC" && e.HasBBox {
59+
catBox = [4]float64{e.West, e.South, e.East, e.North}
60+
}
61+
}
62+
if catBox == ([4]float64{}) {
63+
t.Fatal("catalogue fixture lacks US5MD1MC coverage")
64+
}
65+
66+
meta := ExtractCellMeta(map[string]CellData{"US5MD1MC.000": {Base: data}}, cat, nil)
67+
m, ok := meta["US5MD1MC"]
68+
if !ok {
69+
t.Fatalf("no metadata for US5MD1MC; got %v", keys(meta))
70+
}
71+
if m.Scale != 12000 || m.Agency != 550 {
72+
t.Errorf("identity = scale %d agency %d, want 12000 / 550", m.Scale, m.Agency)
73+
}
74+
if !m.HasBBox || m.BBox != catBox {
75+
t.Errorf("BBox = %v (has=%v), want catalogue box %v verbatim", m.BBox, m.HasBBox, catBox)
76+
}
77+
}
78+
3679
func keys(m map[string]CellMeta) []string {
3780
out := make([]string, 0, len(m))
3881
for k := range m {

internal/engine/server/cellindex.go

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,23 @@ import (
99
"sync"
1010

1111
"github.com/beetlebugorg/chartplotter/internal/engine/baker"
12+
"github.com/beetlebugorg/chartplotter/pkg/s57"
1213
)
1314

15+
// boundsUsable reports whether a parsed cell yielded a non-degenerate bbox (a real
16+
// extent, not the empty/zero box of a cell whose coverage couldn't be derived).
17+
func boundsUsable(b s57.Bounds) bool {
18+
return b.MaxLon > b.MinLon && b.MaxLat > b.MinLat
19+
}
20+
1421
// cellIndex is a small, persistent name→bounding-box index over the cached source
1522
// cells (<dataDir>/ENC_ROOT/<CELL>/<CELL>.000). It lets the server answer "where
1623
// is cell X" and "which installed cells are active" without re-parsing thousands
17-
// of cells on every request: each cell's header is read ONCE (the bbox cached to
18-
// <dataDir>/cells-index.json), then queries hit the in-memory map. Kept
19-
// deliberately simple — a flat JSON map, not a database; the data is tiny (a few
20-
// floats per cell) and read-mostly.
24+
// of cells on every request: each cell is parsed ONCE — only its M_COVR coverage,
25+
// not the whole cell (see scan) — with the bbox cached to <dataDir>/cells-index
26+
// .json, then queries hit the in-memory map. Kept deliberately simple — a flat
27+
// JSON map, not a database; the data is tiny (a few floats per cell) and
28+
// read-mostly.
2129
type cellIndex struct {
2230
mu sync.RWMutex
2331
cond *sync.Cond // broadcast when a scan finishes (for wait())
@@ -126,8 +134,12 @@ func (ci *cellIndex) wait() {
126134
ci.mu.Unlock()
127135
}
128136

129-
// scan reads every cached cell's header once (bbox cached so repeat scans skip the
130-
// already-indexed) and reconciles: drops index entries for cells no longer on disk.
137+
// scan derives every cached cell's bbox once (cached, so repeat scans skip the
138+
// already-indexed) and reconciles: drops index entries for cells no longer on
139+
// disk. The bbox comes from an M_COVR-only coverage parse — the cell's data
140+
// coverage is all we need, so we skip building the geometry, R-tree and portrayal
141+
// of every other feature that a full parse would. A cell with no M_COVR (rare:
142+
// synthetic/test cells) falls back to a full parse so it still gets a bbox.
131143
func (ci *cellIndex) scan() {
132144
entries, err := os.ReadDir(ci.encRoot)
133145
if err != nil {
@@ -148,11 +160,23 @@ func (ci *cellIndex) scan() {
148160
if err != nil {
149161
continue
150162
}
151-
chart, err := baker.ParseCellBytes(name, data)
163+
// M_COVR-only parse: builds just the coverage rings, not every feature's
164+
// geometry — all the bbox needs. nil updates: the index tracks base cells.
165+
chart, err := baker.ParseCellCoverage(name, data, nil)
152166
if err != nil {
153167
continue
154168
}
155169
b := chart.Bounds()
170+
if !boundsUsable(b) {
171+
// No M_COVR coverage polygon (rare — synthetic cells omit it). Fall back to
172+
// a full parse so the cell still lands in the index with a real bbox.
173+
if full, ferr := baker.ParseCellBytes(name, data); ferr == nil {
174+
b = full.Bounds()
175+
}
176+
}
177+
if !boundsUsable(b) {
178+
continue // still nothing usable; skip rather than index a degenerate box
179+
}
156180
ci.mu.Lock()
157181
ci.bbox[name] = [4]float64{b.MinLon, b.MinLat, b.MaxLon, b.MaxLat}
158182
ci.mu.Unlock()

internal/engine/server/import.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ func (s *Server) bakeAndRegister(jobID, set string, cells map[string]baker.CellD
615615
// agency/coverage (cheap coverage-only parse) overlaid with the catalogue's chart
616616
// titles + coverage. Best-effort — a write failure only costs the extracted detail.
617617
s.imports.update(jobID, func(j *importJob) { j.Phase, j.Note = "meta", "Reading chart metadata" })
618-
cellMeta := baker.ExtractCellMeta(cells, func(name string, e error) {
618+
cellMeta := baker.ExtractCellMeta(cells, cat, func(name string, e error) {
619619
log.Printf("import %s: meta skip %s: %v", jobID, name, e)
620620
})
621621
meta := buildSetMeta(set, cellMeta, cat)

internal/engine/server/import_meta_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestImport_NoCatalog(t *testing.T) {
7676
if set := s.deriveUploadSet(cat, cells); set != "user-us5md1mc" {
7777
t.Errorf("deriveUploadSet = %q, want user-us5md1mc", set)
7878
}
79-
meta := buildSetMeta("user-us5md1mc", baker.ExtractCellMeta(cells, nil), cat)
79+
meta := buildSetMeta("user-us5md1mc", baker.ExtractCellMeta(cells, cat, nil), cat)
8080
if meta.ScaleMin != 12000 || len(meta.BBox) != 4 || meta.Agency != "NOAA (US)" {
8181
t.Errorf("header metadata missing: scale=%d bbox=%v agency=%q", meta.ScaleMin, meta.BBox, meta.Agency)
8282
}
@@ -115,7 +115,7 @@ func TestImport_AutoNameAndMeta(t *testing.T) {
115115
}
116116

117117
// The post-bake metadata tail (bakeAndRegister does exactly this after baking).
118-
cellMeta := baker.ExtractCellMeta(cells, nil)
118+
cellMeta := baker.ExtractCellMeta(cells, cat, nil)
119119
meta := buildSetMeta(set, cellMeta, cat)
120120
meta.Imported = "2026-06-25T00:00:00Z"
121121
if err := s.writeSetMeta(set, meta); err != nil {

internal/s57/parser/header.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package parser
2+
3+
import (
4+
"fmt"
5+
"io"
6+
"io/fs"
7+
8+
"github.com/beetlebugorg/chartplotter/pkg/iso8211"
9+
)
10+
11+
// CellHeader is the lightweight identity + compilation scale of an S-57 cell,
12+
// read from only its leading DSID/DSPM records — no feature or spatial records,
13+
// no geometry. It is the cheap answer when a caller needs to know WHAT a cell is
14+
// (and at what scale/band) without portraying it.
15+
//
16+
// Note: S-57 stores NO bounding box in the header. A cell's geographic extent
17+
// comes from its M_COVR coverage features (or the exchange-set catalogue's CATD
18+
// bbox), neither of which is read here. Use the catalogue or an M_COVR-only parse
19+
// for bounds.
20+
type CellHeader struct {
21+
DatasetName string // DSID DSNM — cell code, e.g. "US5MD1MC"
22+
Edition string // DSID EDTN
23+
UpdateNumber string // DSID UPDN ("0" for a base cell)
24+
IssueDate string // DSID ISDT (YYYYMMDD)
25+
ProducingAgency int // DSID AGEN — IHO agency code (550 = NOAA)
26+
CompilationScale int32 // DSPM CSCL — scale denominator (0 if no DSPM)
27+
}
28+
29+
// ReadHeaderFS reads only a cell's leading dataset-metadata records (DSID + DSPM)
30+
// from fsys, stopping as soon as both are seen — or when the metadata block ends
31+
// (the first feature/spatial record) — without ever reading the feature or
32+
// spatial records. This is dramatically cheaper than Parse when only identity and
33+
// scale are needed (e.g. bucketing cells by band, or filling in metadata whose
34+
// bounds come from elsewhere). Updates are NOT applied: the result reflects the
35+
// base cell as given.
36+
func ReadHeaderFS(fsys fs.FS, filename string) (*CellHeader, error) {
37+
p, err := iso8211.OpenFS(fsys, filename)
38+
if err != nil {
39+
return nil, err
40+
}
41+
defer p.Close()
42+
return readHeader(p)
43+
}
44+
45+
func readHeader(p *iso8211.Parser) (*CellHeader, error) {
46+
h := &CellHeader{}
47+
var gotDSID, gotDSPM bool
48+
// DSID and DSPM live in the dataset general-information / geographic-reference
49+
// records at the very front of the file, before any feature (FRID) or spatial
50+
// (VRID) record. Read records one at a time until both are in hand, or until the
51+
// metadata block is over — so a cell that omits DSPM doesn't drag us through the
52+
// whole file.
53+
for !(gotDSID && gotDSPM) {
54+
rec, err := p.Next()
55+
if err == io.EOF {
56+
break
57+
}
58+
if err != nil {
59+
return nil, err
60+
}
61+
if d, ok := rec.Fields["DSID"]; ok && !gotDSID {
62+
m := parseDSID(d)
63+
h.DatasetName = m.dsnm
64+
h.Edition = m.edtn
65+
h.UpdateNumber = m.updn
66+
h.IssueDate = m.isdt
67+
h.ProducingAgency = m.agen
68+
gotDSID = true
69+
}
70+
if d, ok := rec.Fields["DSPM"]; ok && !gotDSPM {
71+
h.CompilationScale = parseDSPM(d).CSCL
72+
gotDSPM = true
73+
}
74+
// First feature/spatial record ⇒ the metadata block has ended; nothing more
75+
// to find. Stop rather than scan the rest of the cell.
76+
if _, ok := rec.Fields["FRID"]; ok {
77+
break
78+
}
79+
if _, ok := rec.Fields["VRID"]; ok {
80+
break
81+
}
82+
}
83+
if !gotDSID {
84+
return nil, fmt.Errorf("no DSID record in cell header")
85+
}
86+
return h, nil
87+
}

0 commit comments

Comments
 (0)