From ffb1b2832e9a4c04321ef336a9370190af4b83dc Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 25 Jun 2026 18:02:55 -0400 Subject: [PATCH 1/7] perf(bake): portray override passes only over consuming geometry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The S-101 portrayer ran the whole cell through the rule engine three times per cell — the default pass plus the PlainBoundaries and SimplifiedSymbols variant passes — but Passes only ever reads the plain variant for polygons and the simplified variant for non-SOUNDG points. Lines and soundings were portrayed three times and their variant builds discarded. Add BuildBatchFiltered, which portrays only the features a predicate selects while still deriving cross-feature context (danger depths, co-located topmarks) from the full feature set, and drive the two override passes with geometry-type predicates that mirror Passes' consumption exactly. Output is unchanged; lines and soundings are now portrayed once. On the golden cell US4MD81M.000 the build/portrayal phase drops ~8.7s→~4.7s (~1.85x), allocations 42.4M→23.7M (-44%), bytes 6.40GB→3.24GB (-49%). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/bake/s101portrayer.go | 23 +++++++++++++++++++++-- internal/engine/portrayal/s101build.go | 19 ++++++++++++++++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/internal/engine/bake/s101portrayer.go b/internal/engine/bake/s101portrayer.go index 1419c7c..0166a5c 100644 --- a/internal/engine/bake/s101portrayer.go +++ b/internal/engine/bake/s101portrayer.go @@ -87,14 +87,33 @@ func (p *s101Portrayer) Begin(features []*s57.Feature) { return } p.cache = m - if v, err := p.builder.BuildBatchOverrides(features, map[string]string{"PlainBoundaries": "true"}); err == nil { + // Each override pass portrays ONLY the geometry type whose variant it + // contributes — PlainBoundaries varies area boundaries (consumed only for + // polygons in Passes), SimplifiedSymbols varies point symbols (consumed only + // for non-SOUNDG points). Lines and soundings never read either variant, so + // portraying them here is wasted rule evaluation. The predicates mirror the + // consumption in Passes exactly, so output is unchanged. + if v, err := p.builder.BuildBatchFiltered(features, map[string]string{"PlainBoundaries": "true"}, isAreaFeature); err == nil { p.plain = v } - if v, err := p.builder.BuildBatchOverrides(features, map[string]string{"SimplifiedSymbols": "true"}); err == nil { + if v, err := p.builder.BuildBatchFiltered(features, map[string]string{"SimplifiedSymbols": "true"}, isSimplifiablePoint); err == nil { p.simplified = v } } +// isAreaFeature selects Surface features — the only consumers of the +// PlainBoundaries (plain area-boundary) variant in Passes. +func isAreaFeature(f *s57.Feature) bool { + return f.Geometry().Type == s57.GeometryTypePolygon +} + +// isSimplifiablePoint selects non-SOUNDG point features — the only consumers of +// the SimplifiedSymbols (simplified point-symbol) variant in Passes. SOUNDG digit +// glyphs don't vary, so they stay a single common pass. +func isSimplifiablePoint(f *s57.Feature) bool { + return f.Geometry().Type == s57.GeometryTypePoint && f.ObjectClass() != "SOUNDG" +} + // End drops the per-cell caches so their memory is released before the next cell. func (p *s101Portrayer) End() { p.cache, p.plain, p.simplified = nil, nil, nil } diff --git a/internal/engine/portrayal/s101build.go b/internal/engine/portrayal/s101build.go index 6bb1bb9..bce57da 100644 --- a/internal/engine/portrayal/s101build.go +++ b/internal/engine/portrayal/s101build.go @@ -82,6 +82,17 @@ func (b *S101Builder) BuildBatch(features []*s57.Feature) (map[int64]FeatureBuil // {"PlainBoundaries":"true"} or {"SimplifiedSymbols":"true"}), so the baker can // portray the plain-boundary / simplified-symbol display variants. func (b *S101Builder) BuildBatchOverrides(features []*s57.Feature, overrides map[string]string) (map[int64]FeatureBuild, error) { + return b.BuildBatchFiltered(features, overrides, nil) +} + +// BuildBatchFiltered is BuildBatchOverrides that portrays only the features for +// which include returns true (nil = all). Cross-feature context (danger depths, +// co-located topmarks) is still derived from the FULL feature set, so filtering +// the portrayed batch never changes a rule's inputs. The override passes use this +// to portray only the geometry type whose variant they contribute — +// PlainBoundaries varies area boundaries, SimplifiedSymbols varies point symbols — +// instead of re-portraying every feature and discarding all but the matching type. +func (b *S101Builder) BuildBatchFiltered(features []*s57.Feature, overrides map[string]string, include func(*s57.Feature) bool) (map[int64]FeatureBuild, error) { eng, err := s101.NewEngineFS(b.rulesFS, b.fcCat) if err != nil { return nil, err @@ -93,6 +104,9 @@ func (b *S101Builder) BuildBatchOverrides(features []*s57.Feature, overrides map topmarkIdx := buildTopmarkIndex(features) batch := make([]s101.Feature, 0, len(features)) for _, f := range features { + if include != nil && !include(f) { + continue + } g := f.Geometry() // Skip non-spatial collection/relationship objects (C_AGGR, C_ASSO) — they // group other features and carry no geometry, so there's nothing to @@ -142,8 +156,11 @@ func (b *S101Builder) BuildBatchOverrides(features []*s57.Feature, overrides map if err != nil { return nil, err } - out := make(map[int64]FeatureBuild, len(features)) + out := make(map[int64]FeatureBuild, len(batch)) for _, f := range features { + if include != nil && !include(f) { + continue + } out[f.ID()] = b.buildFeature(f, streams[strconv.FormatInt(f.ID(), 10)]) } return out, nil From a3e28f6a3b037aa0738a9c57bef2e6d947791899 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 25 Jun 2026 18:15:43 -0400 Subject: [PATCH 2/7] perf(s101): cache compiled Lua chunk prototypes across engines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NewEngineFS re-parsed and re-compiled the whole S-101 framework (and every per-class rule file) from Lua source on every engine construction — three times per cell, plus once per cell again for each distinct rule file. Add a ProtoCache that memoizes *lua.FunctionProto by require name; the builder owns one cache and shares it across every engine it creates, so the framework compiles once per bake instead of ~3x per cell. Each engine still gets a fresh LState (per-cell catalogue caches are freed on Close); only the immutable compiled prototypes are shared. NewFunctionFromProto is an exact drop-in for Load here — top-level chunks have no upvalues and bind globals via ls.Env. Measurement reset expectations: profiling shows compilation is NOT the bottleneck. On an 8-cell district the saving is only ~1-2% time / ~2% allocs, because the dominant cost is per-feature rule *execution* and the Lua-table allocation it drives (gopher-lua RawSetString is 68% of bytes allocated), not chunk compilation. Kept because it's correct, low-risk, and helps the server's live/incremental portrayal and many-small-cell districts more than one big cell. Adds BenchmarkBuildBakerGolden to cover the build/portrayal phase that BenchmarkBakeGoldenParallel skips. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../engine/baker/buildphase_bench_test.go | 16 +++++ internal/engine/portrayal/s101build.go | 19 ++++-- internal/engine/s101/engine.go | 65 +++++++++++++++++-- 3 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 internal/engine/baker/buildphase_bench_test.go diff --git a/internal/engine/baker/buildphase_bench_test.go b/internal/engine/baker/buildphase_bench_test.go new file mode 100644 index 0000000..ebb571f --- /dev/null +++ b/internal/engine/baker/buildphase_bench_test.go @@ -0,0 +1,16 @@ +package baker + +import "testing" + +// BenchmarkBuildBakerGolden measures the parse + portrayal + routing phase +// (BuildBaker) on the golden cell — the phase BenchmarkBakeGoldenParallel +// excludes by resetting its timer after the build. This is where the S-101 rule +// engine runs, so it's the benchmark to watch for portrayal-side optimizations. +func BenchmarkBuildBakerGolden(b *testing.B) { + cells := goldenCellBytes(b) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _, _ = BuildBaker(cells, nil) + } +} diff --git a/internal/engine/portrayal/s101build.go b/internal/engine/portrayal/s101build.go index bce57da..c10cea2 100644 --- a/internal/engine/portrayal/s101build.go +++ b/internal/engine/portrayal/s101build.go @@ -50,13 +50,17 @@ func newS101Builder(catalogFS fs.FS, fcBytes []byte) (*S101Builder, error) { if err != nil { return nil, err } - // Validate the framework loads (fail fast); discard this engine. - eng, err := s101.NewEngineFS(rulesFS, cat) + // One shared prototype cache for every engine this builder creates — the + // framework (and per-class rule chunks) are parsed + compiled once across the + // whole bake instead of three times per cell. Validating the framework here + // (fail fast) also warms the cache with the framework prototypes. + cache := s101.NewProtoCache() + eng, err := s101.NewEngineFSCached(rulesFS, cat, cache) if err != nil { return nil, err } eng.Close() - return &S101Builder{rulesFS: rulesFS, fcCat: cat, Catalog: draw}, nil + return &S101Builder{rulesFS: rulesFS, fcCat: cat, Catalog: draw, protoCache: cache}, nil } // S101Builder is the feature-build seam: it runs the S-101 portrayal rules (via @@ -64,9 +68,10 @@ func newS101Builder(catalogFS fs.FS, fcBytes []byte) (*S101Builder, error) { // instruction stream, and emits a primitive for each draw onto the feature // geometry to produce the Primitive stream the baker consumes. type S101Builder struct { - rulesFS fs.FS - fcCat *fc.Catalogue - Catalog *catalog.Catalog + rulesFS fs.FS + fcCat *fc.Catalogue + Catalog *catalog.Catalog + protoCache *s101.ProtoCache } // BuildBatch portrays a whole cell's features in ONE engine pass (one chunk @@ -93,7 +98,7 @@ func (b *S101Builder) BuildBatchOverrides(features []*s57.Feature, overrides map // PlainBoundaries varies area boundaries, SimplifiedSymbols varies point symbols — // instead of re-portraying every feature and discarding all but the matching type. func (b *S101Builder) BuildBatchFiltered(features []*s57.Feature, overrides map[string]string, include func(*s57.Feature) bool) (map[int64]FeatureBuild, error) { - eng, err := s101.NewEngineFS(b.rulesFS, b.fcCat) + eng, err := s101.NewEngineFSCached(b.rulesFS, b.fcCat, b.protoCache) if err != nil { return nil, err } diff --git a/internal/engine/s101/engine.go b/internal/engine/s101/engine.go index 9ec0855..7f8c000 100644 --- a/internal/engine/s101/engine.go +++ b/internal/engine/s101/engine.go @@ -15,11 +15,52 @@ import ( "io/fs" "os" "strings" + "sync" "github.com/beetlebugorg/chartplotter/pkg/s100/fc" lua "github.com/yuin/gopher-lua" + "github.com/yuin/gopher-lua/parse" ) +// ProtoCache memoizes compiled Lua chunk prototypes by require name so engines +// built from the same Rules tree don't re-parse and re-compile the framework +// (and per-class rule files) from source on every construction. A *FunctionProto +// is immutable after compilation and safe to instantiate into many independent +// LStates via NewFunctionFromProto, so one cache is shared across every engine a +// builder creates. Safe for concurrent use. +type ProtoCache struct { + mu sync.Mutex + protos map[string]*lua.FunctionProto +} + +// NewProtoCache returns an empty prototype cache. +func NewProtoCache() *ProtoCache { return &ProtoCache{protos: map[string]*lua.FunctionProto{}} } + +// proto returns the compiled prototype for .lua under rules, compiling and +// caching it on first request. The lock is held across compile so a cold cache +// compiles each chunk once; after warmup it's a single map read. +func (c *ProtoCache) proto(rules fs.FS, name string) (*lua.FunctionProto, error) { + c.mu.Lock() + defer c.mu.Unlock() + if p, ok := c.protos[name]; ok { + return p, nil + } + data, err := fs.ReadFile(rules, name+".lua") + if err != nil { + return nil, err + } + chunk, err := parse.Parse(bytes.NewReader(data), name+".lua") + if err != nil { + return nil, err + } + p, err := lua.Compile(chunk, name+".lua") + if err != nil { + return nil, err + } + c.protos[name] = p + return p, nil +} + // Feature is one S-57 feature to portray, in S-57 terms. type Feature struct { ID string @@ -99,8 +140,19 @@ func NewEngine(rulesDir string, cat *fc.Catalogue) (*Engine, error) { } // NewEngineFS loads the S-101 framework from an fs.FS rooted at the Rules -// directory (e.g. an embed.FS sub-tree) and binds host callbacks to cat. +// directory (e.g. an embed.FS sub-tree) and binds host callbacks to cat. It +// compiles the framework fresh; reuse a ProtoCache via NewEngineFSCached to share +// compiled chunks across engines. func NewEngineFS(rules fs.FS, cat *fc.Catalogue) (*Engine, error) { + return NewEngineFSCached(rules, cat, NewProtoCache()) +} + +// NewEngineFSCached is NewEngineFS sharing a caller-owned ProtoCache, so a batch +// of engines built from the same Rules tree parse + compile each chunk once +// instead of per engine. Each engine still gets a fresh LState (its per-cell +// catalogue caches are freed on Close); only the immutable compiled prototypes +// are shared. +func NewEngineFSCached(rules fs.FS, cat *fc.Catalogue, cache *ProtoCache) (*Engine, error) { // A growable registry: a single SOUNDG can carry thousands of soundings, and // resolving its multipoint builds one Lua object per point (plus SOUNDG03's // per-point instructions). The default fixed registry overflows on such a @@ -127,15 +179,14 @@ func NewEngineFS(rules fs.FS, cat *fc.Catalogue) (*Engine, error) { return 0 } loaded[name] = true - data, err := fs.ReadFile(rules, name+".lua") - if err != nil { - s.RaiseError("require(%q): %v", name, err) - } - fn, err := s.Load(bytes.NewReader(data), name+".lua") + proto, err := cache.proto(rules, name) if err != nil { s.RaiseError("require(%q): %v", name, err) } - s.Push(fn) + // A top-level chunk has no upvalues and binds globals via ls.Env, so + // NewFunctionFromProto is equivalent to Load here — just without the + // re-parse/compile. + s.Push(s.NewFunctionFromProto(proto)) if err := s.PCall(0, 0, nil); err != nil { s.RaiseError("require(%q): %v", name, err) } From 1d5c61802ed937cedd9c06ff1f0566204a3b50ac Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 25 Jun 2026 18:19:58 -0400 Subject: [PATCH 3/7] perf(portrayal): compute area polylabel anchor only when a draw needs it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildFeature ran textAnchor unconditionally for every feature, and for areas that means areaLabelPoint — the Mapbox polylabel pole-of-inaccessibility search, which scans every edge of every ring per candidate cell. CPU profiling the build/portrayal phase put it at ~21% of total, the single biggest cost, even though most area features (depth areas, land, sea areas) emit only fills and boundary lines that never read the anchor — and areas now run it twice (default + plain-boundary passes). Reduce the draw commands first, then compute the anchor only when a command actually consumes it (point symbols, text, or sector/augmented figures, per commandsNeedAnchor — the anchored ops emitPrimitives reads geom.Anchor for). The centred-area-symbol placement only fires when a SymbolCall exists, i.e. when an OpPoint was emitted and the anchor was computed, so it's unaffected. Golden archive is byte-identical (sha256 3a9566e0…, 3221767 bytes, verified before/after). Build/portrayal phase on the golden cell drops ~4.8s→~3.7s (~22%). Cumulative over this branch the phase is ~8.7s→~3.6s (~2.4x), allocations 42.4M→22.3M (-47%), bytes 6.40GB→3.12GB (-51%). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/portrayal/s101build.go | 28 +++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/internal/engine/portrayal/s101build.go b/internal/engine/portrayal/s101build.go index c10cea2..3a1dc17 100644 --- a/internal/engine/portrayal/s101build.go +++ b/internal/engine/portrayal/s101build.go @@ -196,10 +196,18 @@ func (b *S101Builder) buildFeature(f *s57.Feature, stream string) FeatureBuild { } g := geometryOf(f.Geometry()) - anchor, _ := textAnchor(g) - sg := S101Geometry{Anchor: anchor, Rings: g.area, Lines: strokeRunsFor(g)} - cmds, _ := instructions.Reduce(instructions.ParseStream(stream)) + + // The feature anchor (point symbols / text / sector figures) is consumed only + // by anchored draw ops. For an area it's the polylabel representative point — a + // pole-of-inaccessibility search over every edge, the single biggest CPU cost + // in bake portrayal — yet most area features emit only fills and boundary lines + // that never read it. Compute it only when a command actually needs it. + var anchor geo.LatLon + if commandsNeedAnchor(cmds) { + anchor, _ = textAnchor(g) + } + sg := S101Geometry{Anchor: anchor, Rings: g.area, Lines: strokeRunsFor(g)} var prims []Primitive priority := 0 cat := 0 // unset; resolved from the viewing groups the rule emits @@ -285,6 +293,20 @@ func (b *S101Builder) buildFeature(f *s57.Feature, stream string) FeatureBuild { } } +// commandsNeedAnchor reports whether any reduced draw command consumes the +// feature anchor — the anchored ops emitPrimitives reads geom.Anchor for: point +// symbols, text, and sector/augmented figures. Fills and boundary lines don't, +// so an area emitting only those skips the expensive polylabel anchor. +func commandsNeedAnchor(cmds []instructions.DrawCommand) bool { + for _, c := range cmds { + switch c.Op { + case instructions.OpPoint, instructions.OpText, instructions.OpAugmentedLine: + return true + } + } + return false +} + // strokeRunsFor returns the drawable polylines an S-101 line draw strokes for a // feature, honoring S-52 §8.6.2 masking exactly as the S-52 walker does: a line // feature's drawable parts, or — for an area — its drawable boundary, each with From b75bc59e387a679487dacc90dd8dfd50fe4a5f02 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 25 Jun 2026 18:41:27 -0400 Subject: [PATCH 4/7] perf(bake): parse + portray cells in parallel, route serially MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profiling established the bake's dominant phase (BuildBaker) is single-threaded: cells were parsed and portrayed in a serial loop, so a district used ~one core for the expensive S-101 rule engine while GC ran on the rest (a CPU profile looked ~50% GC-bound, but that was mark workers on idle cores — GOMAXPROCS=1 floored the phase at ~6-8s with no overlap, =8 at ~4s). Per-cell parse and portrayal are independent and pure; only the route/merge into the Baker is stateful and order-dependent. Split portrayal from routing: s101Portrayer gains a pure, concurrency-safe portray() (returns the three pass maps without touching active state) plus install(); Begin = install(portray()). Baker exposes PortrayCell (parallel-safe, mutates nothing) and AddCellPortrayed (the existing serial route, replaying a precomputed portrayal). BuildBaker / BuildBakerWithUpdates now drive a bounded ordered pipeline (addCellsParallel): NumCPU workers parse+portray ahead while the main goroutine routes in sorted cell order. The entire stateful merge stays serial and ordered, so output is byte-for-byte identical. Verified: golden archive sha256 unchanged (3a9566e0…, single cell == serial); multi-cell builds deterministic run-to-run (new TestMultiCellParallelDeterministic); clean under -race across baker/bake/portrayal. 8-cell district build ~25.1s→~10.0s (~2.5x on 8 cores) with identical allocations; scales with district size. Memory: at most ~NumCPU cells are resident at once (the trade for the speedup). The per-band streaming bake remains the path for memory-bounded huge districts. This also makes the next lever — cutting the gopher-lua per-feature table churn (~70% of allocations) — pay off in wall-clock, since GC now contends for cores. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/bake/bake.go | 66 ++++++++++++++++++---- internal/engine/bake/s101portrayer.go | 43 ++++++++++++-- internal/engine/baker/baker.go | 78 +++++++++++++++++++------- internal/engine/baker/parallel_test.go | 33 +++++++++++ 4 files changed, 185 insertions(+), 35 deletions(-) create mode 100644 internal/engine/baker/parallel_test.go diff --git a/internal/engine/bake/bake.go b/internal/engine/bake/bake.go index 1d18881..0fdb771 100644 --- a/internal/engine/bake/bake.go +++ b/internal/engine/bake/bake.go @@ -564,9 +564,49 @@ func (b *Baker) ScaminValues() []uint32 { return out } +// CellPortrayal carries a cell's precomputed portrayal from the parallel +// PortrayCell step to the serial AddCellPortrayed step. It is opaque to callers, +// which only shuttle it between the two; supported is false when the portrayer +// can't precompute (then AddCellPortrayed portrays inline, like AddCell). +type CellPortrayal struct { + cp cellPortrayal + supported bool +} + +// PortrayCell runs a cell's (expensive) S-101 portrayal without mutating the +// Baker, so callers can do it on a worker pool. The result is replayed by +// AddCellPortrayed during the serial route/merge. Concurrency-safe: it only reads +// the read-only portrayer/catalogue and the internally-locked proto cache. +func (b *Baker) PortrayCell(chart *s57.Chart) CellPortrayal { + pp, ok := b.portrayer.(precomputingPortrayer) + if !ok { + return CellPortrayal{} + } + return CellPortrayal{cp: pp.portray(featurePtrs(chart.Features())), supported: true} +} + +// featurePtrs adapts a cell's feature slice to the []*Feature the portrayer takes. +func featurePtrs(features []s57.Feature) []*s57.Feature { + fps := make([]*s57.Feature, len(features)) + for i := range features { + fps[i] = &features[i] + } + return fps +} + // AddCell expands every feature of a parsed cell into routed primitives at the -// cell's scale band, with per-feature SCAMIN display z-min. -func (b *Baker) AddCell(chart *s57.Chart) { +// cell's scale band, with per-feature SCAMIN display z-min. It portrays the cell +// inline; use PortrayCell + AddCellPortrayed to portray off-thread in parallel. +func (b *Baker) AddCell(chart *s57.Chart) { b.addCell(chart, CellPortrayal{}) } + +// AddCellPortrayed is AddCell with the cell's portrayal already computed (by +// PortrayCell, typically on a worker pool). The routing/merge it does is the same +// serial, deterministic work as AddCell — call it in a fixed cell order. +func (b *Baker) AddCellPortrayed(chart *s57.Chart, pc CellPortrayal) { b.addCell(chart, pc) } + +// addCell is the shared body: portray (replaying pc if precomputed, else inline) +// then route the cell's features serially. +func (b *Baker) addCell(chart *s57.Chart, pc CellPortrayal) { if b.portrayer == nil { panic("bake: no S-101 portrayer set — build with `make` (-tags embed_s101) or pass --s101") } @@ -607,14 +647,20 @@ func (b *Baker) AddCell(chart *s57.Chart) { } // S-101: portray the whole cell in one engine pass (one Lua chunk + context, // fresh Lua state) up front instead of per-feature — the per-feature path - // recompiled the chunk and leaked the catalogue's file-local caches. - if bp, ok := b.portrayer.(BatchPortrayer); ok { - fps := make([]*s57.Feature, len(features)) - for i := range features { - fps[i] = &features[i] - } - bp.Begin(fps) - defer bp.End() + // recompiled the chunk and leaked the catalogue's file-local caches. Either + // replay a precomputed portrayal (parallel bake) or portray inline now; the + // result is released after this cell's features are routed. + switch pp := b.portrayer.(type) { + case precomputingPortrayer: + if pc.supported { + pp.install(pc.cp) + } else { + pp.install(pp.portray(featurePtrs(features))) + } + defer pp.End() + case BatchPortrayer: + pp.Begin(featurePtrs(features)) + defer pp.End() } // Combine co-located lights (S-52 LIGHTS06): one flare + one merged label. diff --git a/internal/engine/bake/s101portrayer.go b/internal/engine/bake/s101portrayer.go index 0166a5c..b772fb0 100644 --- a/internal/engine/bake/s101portrayer.go +++ b/internal/engine/bake/s101portrayer.go @@ -25,6 +25,18 @@ type BatchPortrayer interface { End() } +// precomputingPortrayer is a BatchPortrayer that can portray a cell off the +// routing thread — portray is concurrency-safe and returns an opaque result that +// install replays onto the active state before that cell's serial routing. This +// lets the baker portray cells in parallel (the dominant bake cost) while keeping +// the stateful routing/merge serial and deterministic. The S-101 portrayer +// implements it; other portrayers fall back to the serial Begin/End path. +type precomputingPortrayer interface { + BatchPortrayer + portray(features []*s57.Feature) cellPortrayal + install(cp cellPortrayal) +} + // SetPortrayer installs the portrayal engine on the Baker. Set the S-101 // portrayer (NewS101Portrayer) to bake with S-101 symbology. Set before AddCell. func (b *Baker) SetPortrayer(p Portrayer) { b.portrayer = p } @@ -76,17 +88,30 @@ func NewS101PortrayerFS(catalogFS fs.FS, featureCatalogueXML []byte) (Portrayer, return &s101Portrayer{builder: bld}, nil } +// cellPortrayal is one cell's three portrayal-pass result maps. portray produces +// it off the routing thread; install replays it onto the active state just before +// that cell's serial routing — so cells can be portrayed in parallel and merged +// serially (deterministically). +type cellPortrayal struct { + cache, plain, simplified map[int64]portrayal.FeatureBuild +} + // Begin portrays the whole cell up front and caches results. It runs the default // pass plus the PlainBoundaries / SimplifiedSymbols variant passes so the client // can toggle boundary + point-symbol style live. Variant passes are best-effort: // if one fails, that axis degrades to the common pass. -func (p *s101Portrayer) Begin(features []*s57.Feature) { +func (p *s101Portrayer) Begin(features []*s57.Feature) { p.install(p.portray(features)) } + +// portray computes the cell's three portrayal passes WITHOUT touching the +// portrayer's active state, so different cells can be portrayed concurrently: +// each pass builds its own Lua engine and the builder is read-only (the proto +// cache is internally locked). The result is replayed by install. +func (p *s101Portrayer) portray(features []*s57.Feature) cellPortrayal { m, err := p.builder.BuildBatch(features) if err != nil { - p.cache = nil // Passes falls back to per-feature builds - return + return cellPortrayal{} // empty → Passes falls back to per-feature builds } - p.cache = m + cp := cellPortrayal{cache: m} // Each override pass portrays ONLY the geometry type whose variant it // contributes — PlainBoundaries varies area boundaries (consumed only for // polygons in Passes), SimplifiedSymbols varies point symbols (consumed only @@ -94,11 +119,17 @@ func (p *s101Portrayer) Begin(features []*s57.Feature) { // portraying them here is wasted rule evaluation. The predicates mirror the // consumption in Passes exactly, so output is unchanged. if v, err := p.builder.BuildBatchFiltered(features, map[string]string{"PlainBoundaries": "true"}, isAreaFeature); err == nil { - p.plain = v + cp.plain = v } if v, err := p.builder.BuildBatchFiltered(features, map[string]string{"SimplifiedSymbols": "true"}, isSimplifiablePoint); err == nil { - p.simplified = v + cp.simplified = v } + return cp +} + +// install replays a precomputed portrayal onto the active state Passes reads. +func (p *s101Portrayer) install(cp cellPortrayal) { + p.cache, p.plain, p.simplified = cp.cache, cp.plain, cp.simplified } // isAreaFeature selects Surface features — the only consumers of the diff --git a/internal/engine/baker/baker.go b/internal/engine/baker/baker.go index bafde17..9d2bea6 100644 --- a/internal/engine/baker/baker.go +++ b/internal/engine/baker/baker.go @@ -141,19 +141,68 @@ func BuildBaker(cells map[string][]byte, onSkip func(name string, err error)) (* b := bake.New() applyPortrayer(b) - var ok []string - for _, name := range names { - chart, err := ParseCellBytes(name, cells[name]) - if err != nil { + ok := addCellsParallel(b, names, func(name string) (*s57.Chart, error) { + return ParseCellBytes(name, cells[name]) + }, onSkip) + return b, ok, nil +} + +// addCellsParallel parses and portrays cells on a bounded worker pool, then +// routes them into b serially in `names` order. Parsing and S-101 portrayal are +// the dominant bake cost and are independent per cell, so they run concurrently; +// the stateful route/merge stays single-threaded and ordered, so the archive is +// byte-for-byte identical to the serial path. At most ~NumCPU cells are resident +// at once (the trade for the speedup — the per-band streaming bake remains the +// path for memory-bounded huge districts). onSkip fires in `names` order. +func addCellsParallel(b *bake.Baker, names []string, parse func(name string) (*s57.Chart, error), onSkip func(name string, err error)) []string { + workers := runtime.NumCPU() + if workers > len(names) { + workers = len(names) + } + if workers < 1 { + return nil + } + type result struct { + chart *s57.Chart + pc bake.CellPortrayal + err error + } + slots := make([]chan result, len(names)) + for i := range slots { + slots[i] = make(chan result, 1) + } + // sem bounds in-flight cells; a token is acquired before a worker starts and + // released by the consumer after that cell is routed, so no more than `workers` + // parsed+portrayed cells are resident ahead of the routing goroutine. + sem := make(chan struct{}, workers) + go func() { + for i, name := range names { + sem <- struct{}{} + go func(i int, name string) { + chart, err := parse(name) + if err != nil { + slots[i] <- result{err: err} + return + } + slots[i] <- result{chart: chart, pc: b.PortrayCell(chart)} + }(i, name) + } + }() + ok := make([]string, 0, len(names)) + for i, name := range names { + r := <-slots[i] + if r.err != nil { if onSkip != nil { - onSkip(name, err) + onSkip(name, r.err) } + <-sem continue } - b.AddCell(chart) + b.AddCellPortrayed(r.chart, r.pc) ok = append(ok, name) + <-sem } - return b, ok, nil + return ok } // CellData is a base cell (.000) plus its sequential update files (.001, .002, …) @@ -197,19 +246,10 @@ func BuildBakerWithUpdates(cells map[string]CellData, overzoom bool, onSkip func b := bake.New() applyPortrayer(b) b.OverzoomAllBands = overzoom - var ok []string - for _, name := range names { + ok := addCellsParallel(b, names, func(name string) (*s57.Chart, error) { cd := cells[name] - chart, err := ParseCellWithUpdates(name, cd.Base, cd.Updates) - if err != nil { - if onSkip != nil { - onSkip(name, err) - } - continue - } - b.AddCell(chart) - ok = append(ok, name) - } + return ParseCellWithUpdates(name, cd.Base, cd.Updates) + }, onSkip) return b, ok, nil } diff --git a/internal/engine/baker/parallel_test.go b/internal/engine/baker/parallel_test.go new file mode 100644 index 0000000..6d8d673 --- /dev/null +++ b/internal/engine/baker/parallel_test.go @@ -0,0 +1,33 @@ +package baker + +import ( + "crypto/sha256" + "fmt" + "testing" +) + +// TestMultiCellParallelDeterministic guards the property the parallel bake relies +// on: cells are parsed and portrayed concurrently (completing out of order) but +// routed serially in sorted order, so the archive is identical run to run. A +// single-cell bake (TestParallelDeterministic) only runs one worker and doesn't +// exercise the ordered merge, so this uses several cells. +func TestMultiCellParallelDeterministic(t *testing.T) { + one := goldenCellBytes(t)["US4MD81M.000"] + cells := map[string][]byte{} + for i := 0; i < 4; i++ { + cells[fmt.Sprintf("US4MD81M_%02d.000", i)] = one + } + var first string + for run := 0; run < 2; run++ { + b, ok, _ := BuildBaker(cells, nil) + if len(ok) != len(cells) { + t.Fatalf("run %d: routed %d/%d cells", run, len(ok), len(cells)) + } + h := fmt.Sprintf("%x", sha256.Sum256(BakeToPMTiles(b, nil).Finish())) + if run == 0 { + first = h + } else if h != first { + t.Fatalf("nondeterministic parallel merge: run %d hash %s != run0 %s", run, h, first) + } + } +} From e7a58cc71dc4ef7bec155f70f9fe7926f110b194 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 25 Jun 2026 18:53:26 -0400 Subject: [PATCH 5/7] perf(portrayal): memoize the S-101 rule run by feature input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dominant remaining bake cost is the gopher-lua rule execution — not its raw interpreter cycles (~7% of CPU) but the per-operation LTable/LValue heap allocation its design forces (LTable.RawSetString is ~70% of all bytes allocated, driving ~45% of CPU into GC). We can't cut that without abandoning cgo-free (no LuaJIT) or editing the vendored IHO catalogue Lua. So instead skip the rule entirely for duplicate inputs. The rule produces a geometry-INDEPENDENT instruction stream — buildFeature attaches each feature's own geometry afterward — so two features with identical inputs (class, primitive, simple + derived + topmark attributes, multipoint vertices) portray identically. Derived (location-aware danger depth) and Topmark are in the key, so depth/context-dependent features never wrongly merge. BuildBatchFiltered now dedupes the batch by portrayalSignature before eng.Portray and shares each representative's stream across its duplicates. ENC cells repeat inputs heavily: the golden cell has 7334 features but only 1530 distinct signatures — 79% of rule runs skipped. Output is byte-identical (golden sha256 3a9566e0…, verified) and the bake content tests + portrayal -race pass. Single-cell build/portrayal phase ~3.6s→~2.28s (~37%), allocations 22.3M→14.8M (-34%), bytes 3.12GB→1.82GB (-42%) — so it cuts memory as well as time. Cumulative over the branch the phase is ~8.7s→~2.28s (~3.8x), allocations -65%, bytes -72%, plus ~2.5x from parallelism on multi-cell districts. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/portrayal/s101build.go | 80 ++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/internal/engine/portrayal/s101build.go b/internal/engine/portrayal/s101build.go index 3a1dc17..3d0d293 100644 --- a/internal/engine/portrayal/s101build.go +++ b/internal/engine/portrayal/s101build.go @@ -4,6 +4,7 @@ import ( "io/fs" "math" "os" + "sort" "strconv" "strings" @@ -108,6 +109,17 @@ func (b *S101Builder) BuildBatchFiltered(features []*s57.Feature, overrides map[ depthIdx := BuildDepthIndex(features) // underlying DEPARE/DRGARE for danger depths topmarkIdx := buildTopmarkIndex(features) batch := make([]s101.Feature, 0, len(features)) + // Memoize the rule run by portrayal input. The S-101 rule produces a + // geometry-INDEPENDENT instruction stream (the geometry is attached per-feature + // in buildFeature), so two features with identical inputs — class, primitive, + // simple/derived/topmark attributes, multipoint vertices — yield the same + // stream. ENC cells repeat the same inputs across thousands of features + // (coastline, land regions, depth areas), and the gopher-lua rule run is the + // dominant, allocation-heavy bake cost, so run it once per distinct input and + // share the stream. sigRep maps a signature to its representative feature ID; + // repID maps every portrayed feature to the representative whose stream it uses. + sigRep := make(map[string]string) + repID := make(map[int64]string, len(features)) for _, f := range features { if include != nil && !include(f) { continue @@ -135,8 +147,8 @@ func (b *S101Builder) BuildBatchFiltered(features []*s57.Feature, overrides map[ // feature.Spatial would otherwise hit the framework's GetSpatial infinite // recursion (it reads self['Spatial'] right after assigning it nil, which // re-fires __index) — the cause of the OBSTRN/WRECKS stack overflows. - if f.Geometry().Type == s57.GeometryTypePoint { - points = soundingPoints(f.Geometry()) + if g.Type == s57.GeometryTypePoint { + points = soundingPoints(g) if f.ObjectClass() == "SOUNDG" { prim = "MultiPoint" } @@ -147,7 +159,7 @@ func (b *S101Builder) BuildBatchFiltered(features []*s57.Feature, overrides map[ topmark = topmarkIdx[key] } } - batch = append(batch, s101.Feature{ + sf := s101.Feature{ ID: strconv.FormatInt(f.ID(), 10), ObjectClass: f.ObjectClass(), Primitive: prim, @@ -155,22 +167,78 @@ func (b *S101Builder) BuildBatchFiltered(features []*s57.Feature, overrides map[ Derived: DerivedAttrs(f, depthIdx), Points: points, Topmark: topmark, - }) + } + sig := portrayalSignature(&sf) + if rep, ok := sigRep[sig]; ok { + repID[f.ID()] = rep // identical inputs already portrayed; share its stream + continue + } + sigRep[sig] = sf.ID + repID[f.ID()] = sf.ID + batch = append(batch, sf) } streams, err := eng.Portray(batch) if err != nil { return nil, err } - out := make(map[int64]FeatureBuild, len(batch)) + out := make(map[int64]FeatureBuild, len(features)) for _, f := range features { if include != nil && !include(f) { continue } - out[f.ID()] = b.buildFeature(f, streams[strconv.FormatInt(f.ID(), 10)]) + // repID[f.ID()] is "" for the skipped (TOPMAR / non-spatial) features, and + // streams[""] is "" — buildFeature then suppresses them, as before. + out[f.ID()] = b.buildFeature(f, streams[repID[f.ID()]]) } return out, nil } +// portrayalSignature serializes everything the S-101 rules read from a feature — +// class, primitive type, simple + derived + topmark attributes, and any +// multipoint vertices — into a stable key. Features with equal signatures produce +// the same instruction stream, so the rule runs once per distinct signature. NUL +// and unit separators keep distinct field layouts from colliding. +func portrayalSignature(f *s101.Feature) string { + var b strings.Builder + b.WriteString(f.ObjectClass) + b.WriteByte(0) + b.WriteString(f.Primitive) + b.WriteByte(0) + writeSortedAttrs(&b, f.Attributes) + b.WriteByte(0) + writeSortedAttrs(&b, f.Derived) + b.WriteByte(0) + writeSortedAttrs(&b, f.Topmark) + for _, p := range f.Points { + b.WriteByte(0) + b.WriteString(strconv.FormatFloat(p[0], 'g', -1, 64)) + b.WriteByte(',') + b.WriteString(strconv.FormatFloat(p[1], 'g', -1, 64)) + b.WriteByte(',') + b.WriteString(strconv.FormatFloat(p[2], 'g', -1, 64)) + } + return b.String() +} + +// writeSortedAttrs appends an attribute map to b in sorted-key order so the +// signature is stable regardless of map iteration order. +func writeSortedAttrs(b *strings.Builder, m map[string]string) { + if len(m) == 0 { + return + } + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + b.WriteString(k) + b.WriteByte('=') + b.WriteString(m[k]) + b.WriteByte('\x1f') + } +} + // Build expands one S-57 feature (convenience wrapper over BuildBatch; the bake // path uses BuildBatch per cell). ok is false only on engine failure. func (b *S101Builder) Build(f *s57.Feature) (FeatureBuild, bool) { From be1a658c8035d702730aec157c058e8b7ce7b304 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 25 Jun 2026 19:20:04 -0400 Subject: [PATCH 6/7] perf(bake): parallelize the streaming per-band bake (UI import path) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The portrayal wins reach every bake path (they live in the portrayer, behind AddCell), but the parallel parse+portray only covered the merged BuildBaker path. The UI import bake (POST /api/import) and `chartplotter bake --bands` use BakeToPMTilesBandsStreaming, whose two passes routed cells in serial loops — so they stayed single-threaded on the dominant cost. Extract the bounded ordered pipeline into a generic parseInOrder (parse + parallel-safe precompute in the worker; serial, ordered consume on the caller), and build addCellsParallel on it. The streaming bake now uses it for both passes: pass 1 parses in parallel and merges coverage serially; pass 2 (per band) parses+portrays in parallel via addCellsParallel and routes serially. The route/merge stays serial and ordered, so every band archive is byte-identical. Verified: streaming archives byte-identical to the serial path (combined sha256 cef08076…, 4 cells across both passes); deterministic run-to-run (new TestStreamingBakeDeterministic); clean under -race; server import tests pass. 8-cell streaming bake ~26.4s→~15.3s (~1.7x — lower than the merged path's 2.5x because the streaming path parses each cell twice and pass 1 is parse-only). Peak memory rises to ~NumCPU cells in flight per band (the chosen speed/memory trade); the per-band working set still bounds total residency far below the merged path. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/baker/baker.go | 93 +++++++++++++----------- internal/engine/baker/streamhash_test.go | 38 ++++++++++ 2 files changed, 89 insertions(+), 42 deletions(-) create mode 100644 internal/engine/baker/streamhash_test.go diff --git a/internal/engine/baker/baker.go b/internal/engine/baker/baker.go index 9d2bea6..b788812 100644 --- a/internal/engine/baker/baker.go +++ b/internal/engine/baker/baker.go @@ -147,33 +147,36 @@ func BuildBaker(cells map[string][]byte, onSkip func(name string, err error)) (* return b, ok, nil } -// addCellsParallel parses and portrays cells on a bounded worker pool, then -// routes them into b serially in `names` order. Parsing and S-101 portrayal are -// the dominant bake cost and are independent per cell, so they run concurrently; -// the stateful route/merge stays single-threaded and ordered, so the archive is -// byte-for-byte identical to the serial path. At most ~NumCPU cells are resident -// at once (the trade for the speedup — the per-band streaming bake remains the -// path for memory-bounded huge districts). onSkip fires in `names` order. -func addCellsParallel(b *bake.Baker, names []string, parse func(name string) (*s57.Chart, error), onSkip func(name string, err error)) []string { +// parseInOrder parses cells on a bounded worker pool and delivers each to consume +// serially in `names` order. parse and precompute run in the worker (concurrent, +// for the independent per-cell work — S-57 parse + S-101 portrayal); consume runs +// on the calling goroutine in order, so it may mutate shared baker state and the +// result stays deterministic. At most ~NumCPU cells are resident at once: a token +// is taken before a worker starts and released by the consumer after that cell is +// handled. onSkip fires in `names` order for parse errors. +func parseInOrder[T any]( + names []string, + parse func(name string) (*s57.Chart, error), + precompute func(name string, chart *s57.Chart) T, + consume func(name string, chart *s57.Chart, pre T), + onSkip func(name string, err error), +) { workers := runtime.NumCPU() if workers > len(names) { workers = len(names) } if workers < 1 { - return nil + return } type result struct { chart *s57.Chart - pc bake.CellPortrayal + pre T err error } slots := make([]chan result, len(names)) for i := range slots { slots[i] = make(chan result, 1) } - // sem bounds in-flight cells; a token is acquired before a worker starts and - // released by the consumer after that cell is routed, so no more than `workers` - // parsed+portrayed cells are resident ahead of the routing goroutine. sem := make(chan struct{}, workers) go func() { for i, name := range names { @@ -184,11 +187,10 @@ func addCellsParallel(b *bake.Baker, names []string, parse func(name string) (*s slots[i] <- result{err: err} return } - slots[i] <- result{chart: chart, pc: b.PortrayCell(chart)} + slots[i] <- result{chart: chart, pre: precompute(name, chart)} }(i, name) } }() - ok := make([]string, 0, len(names)) for i, name := range names { r := <-slots[i] if r.err != nil { @@ -198,10 +200,24 @@ func addCellsParallel(b *bake.Baker, names []string, parse func(name string) (*s <-sem continue } - b.AddCellPortrayed(r.chart, r.pc) - ok = append(ok, name) + consume(name, r.chart, r.pre) <-sem } +} + +// addCellsParallel parses and portrays cells on a worker pool (parseInOrder), then +// routes them into b serially in `names` order. Parsing and S-101 portrayal are +// the dominant bake cost and are independent per cell, so they run concurrently; +// the stateful route/merge stays single-threaded and ordered, so the archive is +// byte-for-byte identical to the serial path. Returns the routed cell names. +func addCellsParallel(b *bake.Baker, names []string, parse func(name string) (*s57.Chart, error), onSkip func(name string, err error)) []string { + ok := make([]string, 0, len(names)) + parseInOrder(names, parse, + func(_ string, chart *s57.Chart) bake.CellPortrayal { return b.PortrayCell(chart) }, + func(name string, chart *s57.Chart, pc bake.CellPortrayal) { + b.AddCellPortrayed(chart, pc) + ok = append(ok, name) + }, onSkip) return ok } @@ -354,22 +370,22 @@ func BakeToPMTilesBandsStreaming(cells map[string]CellData, maxZoom uint32, onSk } sort.Strings(names) - // Pass 1: coverage + band per cell (no routing). - byBand := map[uint32][]string{} - parsed := 0 - for _, name := range names { + parse := func(name string) (*s57.Chart, error) { cd := cells[name] - chart, err := ParseCellWithUpdates(name, cd.Base, cd.Updates) - if err != nil { - if onSkip != nil { - onSkip(name, err) - } - continue - } - band := b.AddCellCoverage(chart) - byBand[band.ZoomRange().Max] = append(byBand[band.ZoomRange().Max], name) - parsed++ + return ParseCellWithUpdates(name, cd.Base, cd.Updates) } + + // Pass 1: coverage + band per cell (no routing). Parse in parallel; the + // coverage merge (covMeta) stays serial and ordered. + byBand := map[uint32][]string{} + parsed := 0 + parseInOrder(names, parse, + func(string, *s57.Chart) struct{} { return struct{}{} }, + func(name string, chart *s57.Chart, _ struct{}) { + band := b.AddCellCoverage(chart) + byBand[band.ZoomRange().Max] = append(byBand[band.ZoomRange().Max], name) + parsed++ + }, onSkip) b.SetSkipCoverage(true) // covMeta is now global; don't re-derive it per band // Pass 2: per band, re-parse + route + bake + free. @@ -379,17 +395,10 @@ func BakeToPMTilesBandsStreaming(cells map[string]CellData, maxZoom uint32, onSk continue } b.ResetPrims() - for _, name := range bandCells { - cd := cells[name] - chart, err := ParseCellWithUpdates(name, cd.Base, cd.Updates) - if err != nil { - if onSkip != nil { - onSkip(name, err) - } - continue - } - b.AddCell(chart) - } + // Parse + portray this band's cells in parallel; route them serially in + // order (deterministic, same as the per-cell loop). Coverage is skipped + // (SetSkipCoverage above), so re-parsing is just for this band's geometry. + addCellsParallel(b, bandCells, parse, onSkip) coords := b.TileCoordsBand(MVTExtent, bd.Min, bd.Max) if len(coords) == 0 { continue diff --git a/internal/engine/baker/streamhash_test.go b/internal/engine/baker/streamhash_test.go new file mode 100644 index 0000000..4040a2a --- /dev/null +++ b/internal/engine/baker/streamhash_test.go @@ -0,0 +1,38 @@ +package baker + +import ( + "crypto/sha256" + "fmt" + "sort" + "testing" + + "github.com/beetlebugorg/chartplotter/internal/engine/pmtiles" +) + +// TestStreamingBakeDeterministic guards the parallel streaming bake (the UI +// import path): cells are parsed+portrayed concurrently in both passes but +// merged/routed serially in order, so each band archive is identical run to run. +func TestStreamingBakeDeterministic(t *testing.T) { + one := goldenCellBytes(t)["US4MD81M.000"] + cells := map[string]CellData{} + for i := 0; i < 4; i++ { + cells[fmt.Sprintf("US4MD81M_%02d.000", i)] = CellData{Base: one} + } + bake := func() string { + var lines []string + _, _, err := BakeToPMTilesBandsStreaming(cells, 0, nil, nil, + func(slug string, pb *pmtiles.Builder) error { + arc := pb.Finish() + lines = append(lines, fmt.Sprintf("%s=%x", slug, sha256.Sum256(arc))) + return nil + }) + if err != nil { + t.Fatal(err) + } + sort.Strings(lines) + return fmt.Sprint(lines) + } + if a, b := bake(), bake(); a != b { + t.Fatalf("nondeterministic streaming bake:\n%s\n%s", a, b) + } +} From 4784f0c870e11711a72b417da26c297965454ce2 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 25 Jun 2026 19:45:07 -0400 Subject: [PATCH 7/7] perf(bake): coverage-only parse for the streaming bake's pass 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The streaming bake parses every cell twice — pass 1 to build the global covMeta (best-available suppression + scale boundaries need coverage for ALL bands before any band emits, since a band's suppression looks at finer cells and its scale boundaries look at coarser ones — no single processing order satisfies both, so the global pre-pass can't be folded into the per-band loop). But pass 1 only ever reads M_COVR (extractCoverage ignores everything else), so its full parse built the geometry of thousands of features it never looks at. Add ParseCellCoverage — ParseCellWithUpdates with ObjectClassFilter=["M_COVR"], which the parser applies BEFORE geometry construction, so non-coverage features are skipped entirely (the band still comes from the header scale; updates still apply). Use it for pass 1. Byte-identical (the coverage pass consumed only M_COVR already). On the 8-cell streaming bake: allocations 173M→135M (-22%, ~38M fewer from the skipped geometry construction), bytes 20.6GB→18.8GB (-9%), ~5% faster. The second pass remains the one real full parse. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/baker/baker.go | 39 ++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/internal/engine/baker/baker.go b/internal/engine/baker/baker.go index b788812..6bd7c17 100644 --- a/internal/engine/baker/baker.go +++ b/internal/engine/baker/baker.go @@ -229,10 +229,10 @@ type CellData struct { Updates map[string][]byte } -// ParseCellWithUpdates parses a base cell with its update files applied. The base -// + every update are staged on an in-memory filesystem so the parser discovers -// and applies the .001/.002/… chain (vs ParseCellBytes, which parses base-only). -func ParseCellWithUpdates(name string, base []byte, updates map[string][]byte) (*s57.Chart, error) { +// cellParseOpts stages a base cell + its update files on an in-memory filesystem +// (so the parser discovers and applies the .001/.002/… chain) and returns the +// path + the baker's standard parse options. +func cellParseOpts(name string, base []byte, updates map[string][]byte) (string, s57.ParseOptions) { p := "/" + path.Base(name) fsys := iso8211.MemFS{p: base} dir := path.Dir(p) @@ -243,6 +243,26 @@ func ParseCellWithUpdates(name string, base []byte, updates map[string][]byte) ( opts.Fs = fsys opts.ApplyUpdates = true opts.MaskCoastlineCoincidentBoundaries = true + return p, opts +} + +// ParseCellWithUpdates parses a base cell with its update files applied (vs +// ParseCellBytes, which parses base-only). +func ParseCellWithUpdates(name string, base []byte, updates map[string][]byte) (*s57.Chart, error) { + p, opts := cellParseOpts(name, base, updates) + return s57.ParseWithOptions(p, opts) +} + +// ParseCellCoverage parses ONLY a cell's M_COVR coverage features, skipping every +// other feature's geometry construction (the expensive topology/ring assembly). +// The streaming bake's coverage pass reads only M_COVR (extractCoverage) and the +// cell's band comes from the header scale, so this yields the same covMeta far +// cheaper than a full parse. Updates are still applied (the filter acts after +// them), so the coverage reflects the cell's current edition. +func ParseCellCoverage(name string, base []byte, updates map[string][]byte) (*s57.Chart, error) { + p, opts := cellParseOpts(name, base, updates) + opts.ObjectClassFilter = []string{"M_COVR"} + opts.MaskCoastlineCoincidentBoundaries = false // irrelevant to M_COVR rings; skip the coastline-edge setup return s57.ParseWithOptions(p, opts) } @@ -375,11 +395,16 @@ func BakeToPMTilesBandsStreaming(cells map[string]CellData, maxZoom uint32, onSk return ParseCellWithUpdates(name, cd.Base, cd.Updates) } - // Pass 1: coverage + band per cell (no routing). Parse in parallel; the - // coverage merge (covMeta) stays serial and ordered. + // Pass 1: coverage + band per cell (no routing). Parse in parallel — and only + // the M_COVR coverage (extractCoverage reads nothing else), so the geometry of + // every other feature isn't built here; the full parse happens once, in pass 2. + // The coverage merge (covMeta) stays serial and ordered. byBand := map[uint32][]string{} parsed := 0 - parseInOrder(names, parse, + parseInOrder(names, func(name string) (*s57.Chart, error) { + cd := cells[name] + return ParseCellCoverage(name, cd.Base, cd.Updates) + }, func(string, *s57.Chart) struct{} { return struct{}{} }, func(name string, chart *s57.Chart, _ struct{}) { band := b.AddCellCoverage(chart)