Skip to content

perf(repl): let a submission's parse trees go before its analysis - #1741

Merged
HuiJun merged 4 commits into
developfrom
perf/release-preparse-trees
Oct 30, 2026
Merged

HuiJun merged 4 commits into
developfrom
perf/release-preparse-trees

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Since #1441 a submission's parse trees ([]parsed, one parser and tree per file) were passed on past submissionResult to resolveReferences/pullReferences, so they stayed referenced through the analysis of every load, for the root namespaces an unresolved reference would be looked up by. Before #1441 they died once acceptParsed had read them. The 0.12.0 gate measured the cost as the +31 MiB peak resident set of sysml -validate on the 12 086-element generated model against 0.11.0, and bisected it to #1441 by the heap held at exit (56 → 74.5 MB).

resolveReferences and pullReferences now take the submission's []SourceFile and parse again (parseRoots) only when the analysis left a subject unresolved — the rare case pays a second parse; the common case holds no trees through the analysis. workspaceLoad.parses becomes sources. The lazy walk of #1624 is kept: a submission that resolves parses nothing twice and walks nothing (TestResolvingNothingDoesNotWalkTheText, adapted to pass sources).

Fragment: changes/unreleased/release-preparse-trees.fixed.md.

How it was verified

Memory acceptance, bin/sysml -validate built as make build builds it (CGO_ENABLED=0 -ldflags "-s -w", go1.27.3 for every column), one warm-up run per binary and model, then three cold runs per configuration, interleaved; medians, peak RSS in MiB (the three runs in brackets). Columns: 0.11.0 source · the merge before #1441 (511061d46) · #1441 (c082e554c) · develop (1c6bb1972) · this branch.

Workload 0.11.0 pre-#1441 #1441 develop this branch
generated 12 086, GOGC=100 162 (162,162,162) 163 (157,163,163) 176 (177,176,176) 181 (181,180,194) 177 (177,172,179)
generated 12 086, GOGC=25 146 (146,144,146) 147 (149,147,145) 165 (164,165,165) 166 (165,166,166) 150 (150,150,150)
generated 6 198, GOGC=25 115 114 125 127 118
Apollo 11 (28 files), GOGC=25 152 149 161 163 156
wall, generated 12 086 0.26 s 0.26 s 0.27 s 0.24 s 0.24 s
GOMEMLIMIT=120MiB, generated 12 086 0.28 s · 141 0.27 s · 141 0.46 s · 154 0.44 s · 155 0.25 s · 144
ulimit -v 1200000, generated 12 086 0.26 s · 161, rc 0 0.27 s · 176, rc 0 0.27 s · 173, rc 0 0.25 s · 193, rc 0 0.24 s · 176, rc 0

GOGC=25 keeps the resident set near the live heap and is the row that separates the configurations; at GOGC=100 the collector's pacing spreads single runs by ±10 MiB (develop's 194). Under the memory limit the candidate spends 0.44 s to 0.11.0's 0.28 s and this branch 0.25 s. Under ulimit -v 600000 every binary, 0.11.0 included, fails at start from the runtime's address-space reservation, so 1.2 GB is the cap used.

Six rounds, benchstat, same four revisions, the load rows (bytes and allocations unchanged by this branch, as expected — the trees were garbage either way; what moves is when):

                          │   pre1441    │               m1441                │                dev                 │                fix7                │
                          │     B/op     │     B/op      vs base              │     B/op      vs base              │     B/op      vs base              │
LoadModel/elements=0-8      227.9Ki ± 0%   227.9Ki ± 0%  -0.03% (p=0.002 n=6)   246.8Ki ± 0%  +8.29% (p=0.002 n=6)   246.8Ki ± 0%  +8.29% (p=0.002 n=6)
LoadModel/elements=250-8    13.96Mi ± 0%   13.96Mi ± 0%       ~ (p=0.132 n=6)   14.71Mi ± 0%  +5.38% (p=0.002 n=6)   14.71Mi ± 0%  +5.38% (p=0.002 n=6)
LoadModel/elements=1000-8   55.54Mi ± 0%   55.55Mi ± 0%       ~ (p=0.589 n=6)   58.32Mi ± 0%  +5.01% (p=0.002 n=6)   58.33Mi ± 0%  +5.02% (p=0.002 n=6)
LoadModel/elements=4000-8   222.4Mi ± 0%   222.4Mi ± 0%       ~ (p=0.937 n=6)   233.5Mi ± 0%  +4.99% (p=0.002 n=6)   233.5Mi ± 0%  +5.00% (p=0.002 n=6)
                          │  allocs/op  │  allocs/op   vs base                │  allocs/op   vs base              │  allocs/op   vs base              │
LoadModel/elements=0-8      2.022k ± 0%   2.022k ± 0%       ~ (p=1.000 n=6) ¹   2.155k ± 0%  +6.58% (p=0.002 n=6)   2.155k ± 0%  +6.58% (p=0.002 n=6)
LoadModel/elements=1000-8   533.1k ± 0%   533.1k ± 0%       ~ (p=0.747 n=6)     538.3k ± 0%  +0.98% (p=0.002 n=6)   538.3k ± 0%  +0.98% (p=0.002 n=6)
LoadModel/elements=4000-8   2.113M ± 0%   2.113M ± 0%       ~ (p=0.589 n=6)     2.133M ± 0%  +0.95% (p=0.002 n=6)   2.133M ± 0%  +0.95% (p=0.002 n=6)
                          │    sec/op    │    sec/op     vs base              │    sec/op     vs base              │   sec/op     vs base               │
LoadModel/elements=1000-8   111.6m ± 10%   115.0m ± 19%       ~ (p=0.589 n=6)   106.2m ±  7%       ~ (p=0.093 n=6)   106.3m ± 1%   -4.75% (p=0.002 n=6)
LoadModel/elements=4000-8   482.3m ±  3%   474.0m ± 23%       ~ (p=0.699 n=6)   455.4m ± 14%       ~ (p=0.065 n=6)   458.0m ± 4%   -5.04% (p=0.015 n=6)
REPLLoadFile-8        1.522 ± 4%   1.528 ±  7%       ~ (p=0.485 n=6)   1.408 ± 56%        ~ (p=0.065 n=6)   1.401 ± 5%   -7.94% (p=0.004 n=6)
REPLSubmitSnippet-8   1.180 ± 6%   1.167 ± 26%       ~ (p=0.818 n=6)   1.018 ±  4%  -13.75% (p=0.002 n=6)   1.046 ± 6%  -11.34% (p=0.002 n=6)

(The +5 % B/op of develop over pre-#1441 on every load row is #1478, addressed in its own PR; the LoadModel/0 allocations are #1462, likewise.)

Definition of done on this branch:

$ go build ./... && go vet ./... && gofmt -l .      # clean, gofmt prints nothing
$ go test ./...                                      # every package ok, with two caveats resolved below:
FAIL  tests/identity   TestPilotLibraryXMI: OPENSYSML_REQUIRE_PILOT_LIBRARY_XMI=1 but pilot library XMI not downloaded
FAIL  tests/corpus     TestPSSMSuiteMigration: OPENSYSML_REQUIRE_PSSM_SUITE=1 but the PSSM suite is not downloaded
FAIL  cmd/sysml        panic: test timed out after 10m0s (TestExploreTablesTheSpacecraftRaceWithinItsBudget at 5m19s) — run while six benchmark rounds loaded the machine
$ ./scripts/download-pssm-suite.sh && ./scripts/download-pilot-library-xmi.sh
$ go test -count=1 ./tests/identity -run TestPilotLibraryXMI && go test -count=1 ./tests/corpus -run TestPSSMSuiteMigration
ok    github.com/Open-MBEE/OpenSysML/tests/identity   0.638s
ok    github.com/Open-MBEE/OpenSysML/tests/corpus     2.666s
$ go test -count=1 -timeout 40m ./cmd/sysml
ok    github.com/Open-MBEE/OpenSysML/cmd/sysml   233.784s

The two corpus gates failed only because the measurement VM set the require variables without holding those corpora; fetched, both pass. Nothing in this branch touches identity or migration.

Checklist

  • make test and make lint pass locally — go build ./..., go vet ./..., gofmt -l . empty, go test ./... above
  • Tests added or updated for the change — TestResolvingNothingDoesNotWalkTheText now drives the sources path
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md) — none covers the retention
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved — no gate count moved
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/15408827f8104c7fbbdbd1b2c852ed2c
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/15408827f8104c7fbbdbd1b2c852ed2c?variant=devin
Requested by: @HuiJun

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review October 30, 2026 05:13
devin-ai-integration[bot]

This comment was marked as resolved.

…ll its references

Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 2 commits October 30, 2026 05:21
… rounds that pull references

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…k requires

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@HuiJun
HuiJun merged commit dbf32bc into develop Oct 30, 2026
33 of 54 checks passed
@HuiJun
HuiJun deleted the perf/release-preparse-trees branch October 30, 2026 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant