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 1419c7c..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,23 +88,61 @@ 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 - if v, err := p.builder.BuildBatchOverrides(features, map[string]string{"PlainBoundaries": "true"}); err == nil { - p.plain = v + 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 + // 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 { + cp.plain = v } - if v, err := p.builder.BuildBatchOverrides(features, map[string]string{"SimplifiedSymbols": "true"}); err == nil { - p.simplified = v + if v, err := p.builder.BuildBatchFiltered(features, map[string]string{"SimplifiedSymbols": "true"}, isSimplifiablePoint); err == nil { + 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 +// 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. diff --git a/internal/engine/baker/baker.go b/internal/engine/baker/baker.go index bafde17..6bd7c17 100644 --- a/internal/engine/baker/baker.go +++ b/internal/engine/baker/baker.go @@ -141,19 +141,84 @@ 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 +} + +// 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 + } + type result struct { + chart *s57.Chart + pre T + err error + } + slots := make([]chan result, len(names)) + for i := range slots { + slots[i] = make(chan result, 1) + } + 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, pre: precompute(name, chart)} + }(i, name) + } + }() + 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) - ok = append(ok, name) + consume(name, r.chart, r.pre) + <-sem } - return b, ok, nil +} + +// 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 } // CellData is a base cell (.000) plus its sequential update files (.001, .002, …) @@ -164,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) @@ -178,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) } @@ -197,19 +282,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 } @@ -314,22 +390,27 @@ func BakeToPMTilesBandsStreaming(cells map[string]CellData, maxZoom uint32, onSk } sort.Strings(names) - // Pass 1: coverage + band per cell (no routing). + parse := func(name string) (*s57.Chart, error) { + cd := cells[name] + return ParseCellWithUpdates(name, cd.Base, cd.Updates) + } + + // 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 - for _, name := range names { + parseInOrder(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 - } - band := b.AddCellCoverage(chart) - byBand[band.ZoomRange().Max] = append(byBand[band.ZoomRange().Max], name) - parsed++ - } + 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) + 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. @@ -339,17 +420,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/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/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) + } + } +} 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) + } +} diff --git a/internal/engine/portrayal/s101build.go b/internal/engine/portrayal/s101build.go index 6bb1bb9..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" @@ -50,13 +51,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 +69,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 @@ -82,7 +88,18 @@ 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) { - eng, err := s101.NewEngineFS(b.rulesFS, b.fcCat) + 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.NewEngineFSCached(b.rulesFS, b.fcCat, b.protoCache) if err != nil { return nil, err } @@ -92,7 +109,21 @@ func (b *S101Builder) BuildBatchOverrides(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 + } 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 @@ -116,8 +147,8 @@ func (b *S101Builder) BuildBatchOverrides(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" } @@ -128,7 +159,7 @@ func (b *S101Builder) BuildBatchOverrides(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, @@ -136,7 +167,15 @@ func (b *S101Builder) BuildBatchOverrides(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 { @@ -144,11 +183,62 @@ func (b *S101Builder) BuildBatchOverrides(features []*s57.Feature, overrides map } out := make(map[int64]FeatureBuild, len(features)) for _, f := range features { - out[f.ID()] = b.buildFeature(f, streams[strconv.FormatInt(f.ID(), 10)]) + if include != nil && !include(f) { + continue + } + // 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) { @@ -174,10 +264,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 @@ -263,6 +361,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 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) }