diff --git a/.build.yml b/.build.yml index 1d1cd9f69d4f836cb680de23fb1768024981d3bb..f5c8b49faba9f6e4140b5a3c8004313688483c1a 100644 --- a/.build.yml +++ b/.build.yml @@ -1,6 +1,7 @@ # builds.sr.ht manifest for spec.sr.ht. One linear pipeline: install the cache # helper, assemble the shared SCSS, stamp a version, restore caches, start a -# Postgres in the VM, test, package with abuild, publish, save caches. +# Postgres in the VM, test, package with abuild, publish, save caches, upload +# this build's own coverage and benchmarks to cov.sr.ht and bench.sr.ht. # # The reasoning behind every task lives in docs/ci.md, not here: builds.sr.ht # stores the submitted manifest in a varchar(16384), so a manifest over 16 KiB @@ -26,8 +27,8 @@ # S3 credentials for the cacher CI cache (Garage `docker-cache` bucket), # same pair the bencher and ci-cacher builds use. - 7dde4219-0783-4581-a67d-c94749de3600 # ~/.s3-cache-key-id - 0e5b3530-6f19-4f30-9b73-9339dd382e46 # ~/.s3-cache-key-secret - # A tokens.sr.ht working token carrying artifacts:upload, the same secret the - # sibling services mount. It is what publish_artifacts sends. + # One tokens.sr.ht working token, the same one the siblings mount, carrying + # artifacts:upload, cov:upload and bench:upload. See docs/ci.md#secrets. - c7968415-1a6d-4ca0-a188-150fb7f57b65 # ~/.srht-token sources: - https://git.srht.bigb.es/~bigbes/sr-ht-spec @@ -47,6 +48,16 @@ # drifts from the rest of the instance. BOOTSTRAP_REV is the submodule commit # core.sr.ht pins at that tag; bump the two together. CORE_VER: "0.84.5" BOOTSTRAP_REV: 779ad9f174ea5ab7e755f6df0ec9e5912d67dd16 + # Dogfooding. Both repo names are the `sources:` line read as ~owner/repo. + # docs/ci.md#coverage, docs/ci.md#bench. + COVER_ORIGIN: https://cov.srht.bigb.es + COVER_REPO: "~bigbes/sr-ht-spec" + BENCH_ORIGIN: https://bench.srht.bigb.es + BENCH_REPO: "~bigbes/sr-ht-spec" +# Literal paths relative to $HOME, and not a fallback. docs/ci.md#artifacts. +artifacts: + - cover.out + - bench.txt submitter: git.sr.ht: allow-refs: @@ -170,10 +181,10 @@ exit 1 fi test -z "$(gofmt -l .)" || { gofmt -l .; echo "gofmt: files above need formatting" >&2; exit 1; } go vet ./... - # `make test` and not a bare `go test ./...`: the Makefile is where this - # repository's test command names its -timeout, and a second copy of that - # number here is a second copy to forget. docs/ci.md#test. - make test + # `make cover`, not a bare `go test ./...`: the Makefile names the -timeout + # and the coverage flags, and it is the suites `make test` runs, so the + # profile is a by-product of the gate. docs/ci.md#test. + make cover COVERPROFILE="$HOME/cover.out" - build: | cd "$REPO" # -d: makedepends come from `packages:`. The APKBUILD runs `make css` @@ -256,3 +267,59 @@ # purpose. Without --force an upload skips a key already there, so no # `cacher exists ||` guard is needed. See docs/ci.md#cache_save. cacher dir upload "$KEY_MOD" ~/go/pkg/mod cacher dir upload "$KEY_GOC" ~/.cache/go-build + - coverage: | + # Dogfooding: the profile the test task wrote, POSTed to this instance's + # own cov.sr.ht. Before bench, whose run is minutes. docs/ci.md#coverage. + cd "$REPO" + # Missing or empty is a 400 about a body rather than about the build. + test -s "$HOME/cover.out" || { echo "no ~/cover.out" >&2; exit 1; } + if [ ! -r ~/.srht-token ]; then + echo "no ~/.srht-token: no cov.sr.ht credentials in this build" + echo "the profile is this build's cover.out artifact and is not lost" + exit 0 + fi + # Both ref prefixes stripped (this builds tags too), key is the idempotency + # key, no Content-Type (the service sniffs), set +x so the header stays out + # of the log, --fail-with-body so a rejection is loud and readable. + # docs/ci.md#the-two-requests. + ref="${GIT_REF#refs/heads/}"; ref="${ref#refs/tags/}" + url="$COVER_ORIGIN/api/v1/repos/$COVER_REPO/reports" + url="$url?commit=$(git rev-parse HEAD)&ref=$ref&key=$JOB_ID&job_url=$JOB_URL" + echo "uploading cover.out to $url" + set +x + curl -sS --fail-with-body -X POST \ + -H "Authorization: Bearer $(cat ~/.srht-token)" \ + --data-binary "@$HOME/cover.out" \ + "$url" + echo + - bench: | + # Dogfooding: this service's own benchmarks, to this instance's own + # bench.sr.ht. Last and its own task on purpose, and this VM measures a + # shape rather than a number. docs/ci.md#bench. + cd "$REPO" + # -s so the recipe is not echoed into the body, and a redirect and a cat + # and NOT `| tee` — tee's exit status would let a failed run pass. + make -s bench > "$HOME/bench.txt" + cat "$HOME/bench.txt" + # `go test -bench` matching nothing prints `ok` and exits 0, and an empty + # body is valid benchfmt, so the names are checked. docs/ci.md#the-two-greps + grep -q '^BenchmarkCompare' "$HOME/bench.txt" + grep -q '^BenchmarkLinkPass' "$HOME/bench.txt" + if [ ! -r ~/.srht-token ]; then + echo "no ~/.srht-token: no bench.sr.ht credentials in this build" + echo "the run is above and is this build's bench.txt artifact" + exit 0 + fi + # The coverage request's shape, plus visibility= — which acts only on the + # POST that creates $BENCH_REPO. + ref="${GIT_REF#refs/heads/}"; ref="${ref#refs/tags/}" + url="$BENCH_ORIGIN/api/v1/repos/$BENCH_REPO/runs" + url="$url?commit=$(git rev-parse HEAD)&ref=$ref&key=$JOB_ID&job_url=$JOB_URL" + url="$url&visibility=public" + echo "uploading bench.txt to $url" + set +x + curl -sS --fail-with-body -X POST \ + -H "Authorization: Bearer $(cat ~/.srht-token)" \ + --data-binary "@$HOME/bench.txt" \ + "$url" + echo diff --git a/Makefile b/Makefile index fe64c197ac5a32b2c67c100c35cfc3dfe4dbfa5d..950ee1a06c265be3f41c27273b86646aa8e15f00 100644 --- a/Makefile +++ b/Makefile @@ -78,6 +78,33 @@ test: go test -timeout $(TEST_TIMEOUT) $(PKG) +# The coverage profile cov.sr.ht is fed, and the one place `go test` grows the +# two flags that decide what that profile means. -covermode=atomic records real +# hit counts rather than a set/unset bit, which is what a trend across commits +# is read off; COVERPROFILE is a variable because CI writes it to $HOME (where +# `artifacts:` looks) while a checkout wants it in the checkout. +# +# It runs exactly the suites `test` runs, so CI runs one of the two and not +# both. +COVERPROFILE?=cover.out + +cover: + go test -timeout $(TEST_TIMEOUT) -covermode=atomic -coverprofile="$(COVERPROFILE)" $(PKG) + go tool cover -func="$(COVERPROFILE)" | tail -1 + +# BENCH_COUNT is `go test -count` for the `bench` target. bench.sr.ht marks a +# point measured under six repetitions "low n" — a point's confidence interval +# only becomes finite at six — so ten is what an uploaded run carries. It is a +# variable so a checkout can say `make bench BENCH_COUNT=1` when it only wants +# to know that the benchmarks still run. +BENCH_COUNT?=10 + +# -run='^$$' so no test runs beside the benchmarks: a run that also executed the +# suites would charge their wall clock to the benchmark task and, in CI, would +# need the Postgres the suites need. +bench: + go test -timeout $(TEST_TIMEOUT) -run='^$$' -bench=. -benchmem -count=$(BENCH_COUNT) $(PKG) + # CSS pipeline: sassc -> minify -> content-hashed filename. The running service # globs web/static/main.min.*.css at startup, so the hash in the name is the # cache-busting version. Requires the shared scss partials installed at @@ -172,5 +199,5 @@ clean: rm -f $(BIN) $(MIGRATE_BIN) rm -f web/static/main.css web/static/main.min.*.css -.PHONY: all build test css run-dev install install-files check-css clean \ - $(BIN) $(MIGRATE_BIN) +.PHONY: all build test cover bench css run-dev install install-files check-css \ + clean $(BIN) $(MIGRATE_BIN) diff --git a/doc/bench_test.go b/doc/bench_test.go new file mode 100644 index 0000000000000000000000000000000000000000..cb090dcbcabfe37854524500937227c2b812eb53 --- /dev/null +++ b/doc/bench_test.go @@ -0,0 +1,256 @@ +package doc + +import ( + "crypto/sha1" + "fmt" + "strings" + "testing" + + "github.com/go-git/go-git/v5/plumbing" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" + "sourcecraft.dev/bigbes/sr-ht-spec/gitx" +) + +// --- what these measure -------------------------------------------------------- +// +// Serving one revision of a space is two pieces of work over the whole corpus, +// and neither of them touches git: FromDocuments turns the blobs a caller +// already read into an Archive — headers parsed, ids contested, hierarchy +// linked, aliases and stems indexed — and LinkPass renders every document +// through the markdown renderer to fill in the link graph the backlinks, +// orphan and catalog views read. +// +// Both grow with the corpus rather than with the request, which is exactly why +// they are worth a number: a space that doubles in size doubles what every page +// of it costs, and nothing about a single document says so. +// +// The corpus below is the shape a specification space actually has: sections of +// documents with frontmatter ids, a parent hierarchy, aliases, wikilinks that +// resolve and wikilinks that do not, relative links, tables and fenced blocks. + +// benchSections and benchPerSection give benchDocs documents in all — a space +// larger than any this instance holds today, so the number says what growth +// costs rather than what today costs. +const ( + benchSections = 8 + benchPerSection = 25 + benchDocs = benchSections * benchPerSection +) + +var benchSpace = core.SpaceRef{Owner: "bigbes", Name: "rfcs"} + +// benchRev is a plausible revision string; nothing here resolves it. +const benchRev = "3f786850e387550fdab836ed7e6dc881de23001b" + +// benchBlob derives a stable, distinct blob hash per path. FromDocuments only +// carries it through to Page.Blob, but a shared zero hash across every document +// would be a corpus no git tree could produce. +func benchBlob(path string) plumbing.Hash { + return plumbing.Hash(sha1.Sum([]byte(path))) +} + +// benchDocuments builds the corpus: one index per section plus benchPerSection +// documents under it, each carrying a header, prose, links and a code block. +func benchDocuments() []gitx.Document { + docs := make([]gitx.Document, 0, benchDocs+benchSections) + + add := func(path, body string) { + docs = append(docs, gitx.Document{ + Path: path, + Blob: benchBlob(path), + Data: []byte(body), + }) + } + + for s := 0; s < benchSections; s++ { + section := fmt.Sprintf("section-%d", s) + indexPath := section + "/index.md" + add(indexPath, fmt.Sprintf(`--- +id: SPEC-%04d +title: Section %d +status: approved +aliases: + - sec-%d +--- + +# Section %d + +The index of this section. It links every document under it, which is what +makes it a catalog and what keeps its own backlinks out of the orphan count. + +%s +`, 9000+s, s, s, s, sectionIndexList(s))) + + for i := 0; i < benchPerSection; i++ { + n := s*benchPerSection + i + path := fmt.Sprintf("%s/doc-%03d.md", section, i) + add(path, benchDocument(s, i, n)) + } + } + return docs +} + +// sectionIndexList is the bullet list a section index carries: a wikilink per +// document under it. +func sectionIndexList(s int) string { + var b strings.Builder + for i := 0; i < benchPerSection; i++ { + fmt.Fprintf(&b, "- [[doc-%03d]] — the %dth document of this section\n", i, i) + } + return b.String() +} + +// benchDocument is one document of the corpus. +func benchDocument(section, i, n int) string { + var b strings.Builder + fmt.Fprintf(&b, `--- +id: SPEC-%04d +title: Document %d of section %d +status: %s +parent: "[[index]]" +aliases: + - d-%d-%d +tags: + - benchmark + - section-%d +--- + +# Document %d of section %d + +`, n, i, section, []string{"draft", "review", "approved"}[n%3], section, i, section, i, section) + + b.WriteString(`This document is prose of the length a specification chapter has, +hard-wrapped the way one is written, so the renderer walks a paragraph +of several lines rather than a single long one. + +`) + + // Links: two that resolve inside this section, one that resolves in another + // section, one relative link, one external, and one that resolves to nothing + // — which is the case the renderer marks rather than drops, and the case a + // space accumulates as it grows. + fmt.Fprintf(&b, "See [[doc-%03d]] and [[doc-%03d]] for the neighbouring rules, "+ + "and [[index]] for the section itself.\n\n", + (i+1)%benchPerSection, (i+2)%benchPerSection) + fmt.Fprintf(&b, "Across sections: [[../section-%d/doc-%03d]] and the alias [[d-%d-%d]].\n\n", + (section+1)%benchSections, i, (section+1)%benchSections, i) + fmt.Fprintf(&b, "A relative link to [the sibling](doc-%03d.md), an external one to\n"+ + ", and [[a-document-nobody-wrote-%d]] which\n"+ + "resolves to nothing at all.\n\n", (i+3)%benchPerSection, i) + + b.WriteString(`## Requirements + +1. The reader must be able to address a revision by its hash. +2. The writer must not be able to rewrite the approved branch. +3. A proposal must name the revision it was cut from. + +| field | required | note | +| ----- | -------- | ---- | +| id | yes | stable across revisions | +| title | yes | shown in every listing | +| status | no | draft when absent | + +` + "```yaml" + ` +id: SPEC-0000 +title: the example +status: draft +` + "```" + ` + +> A block quote, because a specification always has one. +`) + return b.String() +} + +// benchBodies is the path → raw markdown map LinkPass reads, which is the map +// a caller already holds from the same read that produced the documents. +func benchBodies(docs []gitx.Document) map[string][]byte { + bodies := make(map[string][]byte, len(docs)) + for _, d := range docs { + bodies[d.Path] = d.Data + } + return bodies +} + +// BenchmarkFromDocuments builds the archive: every header parsed, the id +// contest decided over the whole revision, the hierarchy linked, and the alias +// and stem indexes filled. +func BenchmarkFromDocuments(b *testing.B) { + docs := benchDocuments() + + b.ReportAllocs() + for b.Loop() { + arc := FromDocuments(benchSpace, benchRev, docs) + // A build that indexed nothing would be the fastest one here. + if len(arc.All()) != len(docs) { + b.Fatalf("archive holds %d pages, want %d", len(arc.All()), len(docs)) + } + } +} + +// BenchmarkLinkPass renders every document in the archive through the markdown +// renderer and resolves its links — the pass that fills the link graph the +// backlink and orphan views read. +func BenchmarkLinkPass(b *testing.B) { + docs := benchDocuments() + bodies := benchBodies(docs) + r := NewRenderer() + + b.ReportAllocs() + for b.Loop() { + b.StopTimer() + // A fresh archive per iteration: LinkPass writes Page.Links, so reusing + // one would measure the second pass over an already-linked graph. + arc := FromDocuments(benchSpace, benchRev, docs) + b.StartTimer() + + if err := arc.LinkPass(r, bodies); err != nil { + b.Fatalf("link pass: %v", err) + } + linked := 0 + for _, p := range arc.All() { + linked += len(p.Links) + } + if linked == 0 { + b.Fatal("the link pass resolved no outbound link at all") + } + } +} + +// BenchmarkRenderDocument is one document rendered on its own, against the +// archive as resolver — the per-request half of the two passes above, and what +// a reader opening a single page pays. +func BenchmarkRenderDocument(b *testing.B) { + docs := benchDocuments() + arc := FromDocuments(benchSpace, benchRev, docs) + r := NewRenderer() + + // Named rather than indexed: a document from the middle of the corpus, and + // deliberately not a section index — an index carries only wikilinks that + // resolve, so it would leave the missing-link path of the renderer unmeasured. + want := fmt.Sprintf("section-%d/doc-%03d.md", benchSections/2, benchPerSection/2) + var src gitx.Document + for _, d := range docs { + if d.Path == want { + src = d + break + } + } + if src.Data == nil { + b.Fatalf("no document at %s in the corpus", want) + } + _, body := ParseFront(src.Data) + dir := DirOf(src.Path) + + b.ReportAllocs() + b.SetBytes(int64(len(body))) + for b.Loop() { + res := r.Render(body, dir, arc) + if len(res.LinkedIDs) == 0 { + b.Fatal("the document resolved no outbound link; the resolver is not being exercised") + } + if len(res.MissingWikilinks) == 0 { + b.Fatal("the deliberately unresolvable wikilink resolved; the fixture drifted") + } + } +} diff --git a/docs/ci.md b/docs/ci.md index 3b45366fb1eb494ce5554ee2e4cd536c8775edd1..07e7c1a5d60525e3fb9f576336adbf06d706ba82 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -8,8 +8,9 @@ Rationale therefore lives here, and the manifest carries pointers. The pipeline is one linear job on `alpine/edge`: install the cache helper, assemble the shared SCSS, stamp a version, restore the Go caches, start a -Postgres, test, package with `abuild`, publish the apk to -`repo.bigb.es/alpine/v3.22/bigbes`, save the caches. +Postgres, test (with coverage), package with `abuild`, publish the apk to +`repo.bigb.es/alpine/v3.22/bigbes`, save the caches, upload the coverage profile +to cov.sr.ht and this build's own benchmarks to bench.sr.ht. It is triggered by a push to the **sourcehut** side. A push to sourcecraft cannot reach builds.sr.ht; the gitsync mirror is what puts the commit on @@ -37,7 +38,7 @@ |---|---|---| | `apk-ci-s3` | `~/.apk-ci.env` | `publish` | | `7dde4219-…` | `~/.s3-cache-key-id` | `cacher_init` | | `0e5b3530-…` | `~/.s3-cache-key-secret` | `cacher_init` | -| `c7968415-…` | `~/.srht-token` | `publish_artifacts` | +| `c7968415-…` | `~/.srht-token` | `publish_artifacts`, `coverage`, `bench` | They are file secrets. Listing them is what turns `publish` and the cache tasks on; a manual submission that asks for no secrets still runs the interesting part @@ -45,6 +46,36 @@ of the pipeline, and `publish` says so and exits 0 rather than failing. `apk-ci-s3` is referenced by name and the other two by UUID, which is only because that is how they were written in the donor manifests; both forms work. + +`~/.srht-token` holds a tokens.sr.ht **working token**, and it is one secret +shared with every sibling service rather than a per-service one. That is what +centralising issuance buys: the credential is minted once, for a person, and +carries the grants of every service it is meant to reach — so this one must +carry `artifacts:upload` for `publish_artifacts`, `cov:upload` for `coverage` +and `bench:upload` for `bench`. Grants are compared literally, so a token +missing one of the three fails that one task and no other; there is no partial +credit and no fallback. + +## environment + +`COVER_REPO` and `BENCH_REPO` are both `~bigbes/sr-ht-spec`, and that is not a +guess from the checkout directory: it is the `sources:` line +(`https://git.srht.bigb.es/~bigbes/sr-ht-spec`) read as `~owner/repo`. The +directory this repository is cloned into is `sourcehut-specs` on at least one +machine — plural, and a different word — which is exactly the name that would +have been wrong. + +## artifacts + +Two, both literal paths relative to `$HOME`: `cover.out` (written by `test`) and +`bench.txt` (written by `bench`). `artifacts:` has no globbing, which is why the +`Makefile` takes `COVERPROFILE` as a variable — CI points it at `$HOME` and a +checkout leaves it in the checkout. + +They are **not** a fallback for the two uploads. They are what a build handed no +secrets still leaves behind, so a manual submission that asked for none can +still be read, and a rejected upload can be replayed by hand from the exact +bytes the build produced. ## cacher @@ -216,13 +247,24 @@ fails the build rather than waiting for someone to notice in review. `gofmt` needs the `test -z "$(gofmt -l .)"` spelling because `gofmt -l` reports the files it would change and still exits 0. -The suite is invoked as `make test` and not as a bare `go test ./...`: the +The suite is invoked as `make cover` and not as a bare `go test ./...`: the Makefile is where this repository's `-timeout` is named (`TEST_TIMEOUT?=20m`), and a second copy of that number here is a second copy to forget. The default `go test` timeout is ten minutes, it is silent about being a default, and what it produces on a slow builder is a goroutine dump rather than a failure anyone can read. +It is `make cover` rather than `make test` because the two run the same suites +over the same tree; `cover` only adds `-covermode=atomic -coverprofile=…`. Doing +it in one task means the profile is a by-product of the gate that already had to +pass, and not a second full run of the suites whose result nothing checks. +`atomic` and not the default `set`: the profile carries real hit counts, which +is what a trend across commits is read off, and `set` would flatten every count +to a bit. `COVERPROFILE="$HOME/cover.out"` because that is where `artifacts:` +looks, and because `abuild` packages this checkout in place: a profile written +into the checkout is one more file in the tree the `version` task had just +proved clean. + ## build `REPODEST=$HOME/packages abuild -d` builds and stages the apk. @@ -345,13 +387,128 @@ a second round-trip that asked the question the upload asks anyway — and one that goes wrong in the direction that matters, since a key that exists but is truncated is exactly the half-restored cache the repair block above is about. +## coverage + +The profile the `test` task already wrote, POSTed to this instance's own +cov.sr.ht. + +It is its own task, and it comes **after** `publish` and `cache_save` for the +reason every upload here does — a rejected report must not cost an apk that was +built, signed and shipped. It comes **before** `bench` because the profile is +already in hand while the benchmark run is minutes long, and a long run has no +business standing between a finished profile and its upload. + +Three things in it are not decoration: + +- `test -s "$HOME/cover.out"` before anything else. A missing or empty profile + is a `test` task that did not write one, and the service would answer that + with a 400 about a body — a message about the request, when the fact is about + the build. +- The `~/.srht-token` gate. Without the file this build was handed no secrets; + that is a manual submission, not a failure, and the profile is still this + build's `cover.out` artifact. **With** the file the upload is fatal on + purpose: a build that has the credential and cannot publish should say so. +- `set +x` immediately before the `curl`. The task runs under `set -x`, and the + `Authorization` header would otherwise be printed into a build log that is + public. + +## bench + +This service's own benchmarks, uploaded to this instance's own bench.sr.ht, +`&visibility=public` so the repository the first POST creates is readable. + +### What is measured, and why those + +Serving one revision of a space is two passes over the whole corpus, and a +proposal view is a third over two whole revisions of a document. None of them is +I/O and all of them grow with the corpus rather than with the request, which is +what makes them worth a number: a space that doubles in size doubles what every +page of it costs, and nothing about a single document says so. + +- `BenchmarkFromDocuments` / `BenchmarkLinkPass` / `BenchmarkRenderDocument` + (`doc/bench_test.go`) run over a synthetic space of 208 documents in 8 + sections — headers with ids and parents, aliases, wikilinks that resolve + inside a section, across sections and through an alias, wikilinks that resolve + to nothing, relative links, tables and fenced blocks. `FromDocuments` is the + archive build (headers parsed, the id contest decided over the whole revision, + the hierarchy linked, the alias and stem indexes filled); `LinkPass` renders + every document to fill the link graph the backlink, orphan and catalog views + read; `RenderDocument` is the single page a reader opens. `LinkPass` rebuilds + the archive per iteration with the timer stopped, because it writes + `Page.Links` and a reused archive would measure a second pass over an + already-linked graph. +- `BenchmarkCompare` / `BenchmarkCompareUnchanged` / `BenchmarkSegment` / + `BenchmarkDiffWords` (`prosediff/bench_test.go`) run the proposal diff over + two revisions of a 24-chapter specification. The edited revision carries one + of every change kind the alignment can recognise — a reworded sentence, an + inserted requirement, a deleted paragraph, an edited code fence and a moved + chapter — so the alignment is not measured on a wall of insertions. + `CompareUnchanged` is a document against itself, which is what most files in a + long proposal are, and `Segment` and `DiffWords` are the two halves alone so a + regression can be attributed rather than guessed at. + +Every one of them asserts its own result inside the loop (the archive holds 208 +pages, the diff found all four change kinds, the render resolved a link *and* +left the deliberately unresolvable wikilink unresolved). A pass that silently +produced nothing would otherwise be the fastest entry in the file — and this is +not hypothetical: the `RenderDocument` assertion caught its own fixture picking +a section index, which carries no unresolvable link, on the first run. + +None of them needs Postgres, which is why this task can sit at the end of the +pipeline without a DSN guard. + +### The two greps + +`go test -bench` that matches nothing prints `ok` and exits `0`, and a file with +no benchmark lines in it is *valid* benchfmt. A renamed or deleted benchmark +would therefore upload an empty run and report success. So the names are checked +against the file before the upload: + +```sh +grep -q '^BenchmarkCompare' "$HOME/bench.txt" +grep -q '^BenchmarkLinkPass' "$HOME/bench.txt" +``` + +One per benchmarked package, so losing either package's benchmarks is loud. + +`make -s bench > "$HOME/bench.txt"` and then `cat`, and deliberately **not** +`| tee`: a pipeline's exit status is the last command's, so `tee` would let a +failing benchmark run pass. `-s` keeps make from echoing the recipe into a file +the parser will read. + +`BENCH_COUNT` is 10. bench.sr.ht marks a point measured under six repetitions +"low n" — a point's confidence interval only becomes finite at six — so a run +uploaded with fewer is a run nobody can read a regression off. A checkout that +only wants to know the benchmarks still run says `make bench BENCH_COUNT=1`. + +A builder VM this small measures a **shape**, not a number: the absolute ns/op +is worth nothing next to a laptop's, and the point of uploading it is that it is +measured the same way every time. + +## The two requests + +Both are a POST with the token as a Bearer header and the file as +`--data-binary`, and both carry the same four query parameters: + +| parameter | value | why | +|---|---|---| +| `commit` | `git rev-parse HEAD` | what the numbers are about | +| `ref` | `$GIT_REF` with both prefixes stripped | see below | +| `key` | `$JOB_ID` | the idempotency key: a resubmitted job replaces, not duplicates | +| `job_url` | `$JOB_URL` | the build a report links back to | + +`ref="${GIT_REF#refs/heads/}"; ref="${ref#refs/tags/}"` strips **both** +prefixes because this pipeline builds tags too (`allow-refs` carries +`refs/tags/v*`), and a tag build would otherwise report `ref=refs/tags/v0.9.0`. +`GIT_REF` is absent altogether on a manually submitted build, which is fine: the +parameter is optional. + +The coverage POST sends **no `Content-Type`** — cov.sr.ht sniffs the body, and a +wrong declared type is worse than none. Both use `curl -sS --fail-with-body`, +which prints the service's JSON error *and* still exits non-zero; plain `--fail` +would swallow the only sentence saying what was wrong. + ## What is not here -- **No coverage upload.** The sibling services POST their profile to - cover.sr.ht at the end of the pipeline. Adding it here needs a token secret - and a repository on cover. -- **No artifacts.** There is nothing to download: the apk goes to S3, and its - name changes every commit, which `artifacts:` cannot express (it has no - globbing). - **No matrix.** One architecture, one image. diff --git a/prosediff/bench_test.go b/prosediff/bench_test.go new file mode 100644 index 0000000000000000000000000000000000000000..5488af64d803b650096ebbc8bdde52960cf5bf48 --- /dev/null +++ b/prosediff/bench_test.go @@ -0,0 +1,171 @@ +package prosediff + +import ( + "fmt" + "strings" + "testing" +) + +// --- what these measure -------------------------------------------------------- +// +// Every proposal view runs Compare over two whole revisions of a specification +// document: both are segmented into blocks with goldmark, the two block +// sequences are aligned, and every block the alignment paired as modified is +// diffed again word by word (or line by line, inside a code fence). It is the +// most expensive thing this service does per request that is not I/O, and it +// grows with the square of the number of blocks in the worst case, so the size +// of the document is the number worth watching. +// +// The corpus below is a specification of the shape this service holds: numbered +// chapters, prose paragraphs, requirement lists, tables and fenced examples. +// The edited revision applies the four edits a proposal actually makes — a +// reworded sentence, an inserted requirement, a deleted paragraph and a moved +// chapter — so the alignment has one of every kind to recognise rather than a +// single wall of insertions. + +// benchChapters is how many chapters the synthetic specification has. Around +// 40 blocks each, so the document is a few hundred blocks: the size at which +// the alignment, and not the segmentation, dominates. +const benchChapters = 24 + +// benchSpec renders a synthetic specification. `shift` rotates the chapter +// bodies by one so that a chapter appears at a different position in the two +// revisions, which is the move the alignment exists to recognise. +func benchSpec(edited bool) []byte { + var b strings.Builder + b.WriteString("---\nid: SPEC-1\ntitle: The synthetic specification\nstatus: draft\n---\n\n") + b.WriteString("# The synthetic specification\n\n") + + order := make([]int, 0, benchChapters) + for i := 0; i < benchChapters; i++ { + order = append(order, i) + } + if edited { + // One chapter moved: the last is read first. Exactly one, so the move + // detection has something to find and the rest of the alignment is still + // an ordinary walk. + order = append(order[len(order)-1:], order[:len(order)-1]...) + } + + for _, i := range order { + fmt.Fprintf(&b, "## %d. Chapter %d\n\n", i+1, i) + fmt.Fprintf(&b, "This chapter states what the service does with the %dth\n"+ + "kind of document, in prose hard-wrapped the way a specification is\n"+ + "written, so that a one-word edit rewraps the paragraph and a\n"+ + "line-oriented differ would report the whole of it as replaced.\n\n", i) + + if edited && i == benchChapters/3 { + // A reworded sentence: same block, edited, so the word-level diff runs. + b.WriteString("A revision **must** carry an id, and the id must be stable\n" + + "across every revision of the document that carries it.\n\n") + } else { + b.WriteString("A revision must carry an id, and that id is stable across\n" + + "every revision of the document that carries it.\n\n") + } + + if !(edited && i == benchChapters/2) { + // Deleted in the edited revision: one whole paragraph gone. + fmt.Fprintf(&b, "The paragraph chapter %d loses when the edit lands. It is\n"+ + "here so the alignment has a deletion to recognise and not only\n"+ + "insertions.\n\n", i) + } + + b.WriteString("Requirements:\n\n") + b.WriteString("- the reader must be able to address a revision by its hash\n") + b.WriteString("- the writer must not be able to rewrite an approved branch\n") + b.WriteString("- a proposal must name the revision it was cut from\n") + if edited && i == benchChapters/4 { + b.WriteString("- a proposal must carry a summary of at most one paragraph\n") + } + b.WriteString("\n") + + b.WriteString("| field | required | note |\n") + b.WriteString("| ----- | -------- | ---- |\n") + b.WriteString("| id | yes | stable across revisions |\n") + b.WriteString("| title | yes | shown in every listing |\n") + b.WriteString("| status | no | draft when absent |\n\n") + + b.WriteString("```yaml\n") + fmt.Fprintf(&b, "id: SPEC-1.%d\n", i) + b.WriteString("title: the example this chapter is about\n") + if edited && i == benchChapters/6 { + // A code fence is diffed line by line, whitespace and all: the other + // half of the split this package exists for. + b.WriteString("status: approved\n") + } else { + b.WriteString("status: draft\n") + } + b.WriteString("```\n\n") + + b.WriteString("> A block quote, because a specification always has one.\n\n") + } + return []byte(b.String()) +} + +// BenchmarkCompare is the proposal view's whole diff: both revisions segmented, +// the block sequences aligned, and every paired block diffed word by word or +// line by line. +func BenchmarkCompare(b *testing.B) { + oldSrc := benchSpec(false) + newSrc := benchSpec(true) + + b.ReportAllocs() + b.SetBytes(int64(len(oldSrc) + len(newSrc))) + for b.Loop() { + d := Compare(oldSrc, newSrc) + // Asserted rather than assumed: an alignment that paired nothing would be + // the fastest run here, and so would one that found no edit at all. + if d.Stats.BlocksModified == 0 || d.Stats.BlocksDeleted == 0 || + d.Stats.BlocksInserted == 0 || d.Stats.BlocksMoved == 0 { + b.Fatalf("the fixture lost a change kind: %+v", d.Stats) + } + } +} + +// BenchmarkCompareUnchanged is the same document against itself — the common +// case in a long proposal, where most files a reviewer opens are untouched. +// What it measures is segmentation plus the alignment's cheap path. +func BenchmarkCompareUnchanged(b *testing.B) { + src := benchSpec(false) + + b.ReportAllocs() + b.SetBytes(int64(2 * len(src))) + for b.Loop() { + d := Compare(src, src) + if d.Stats.BlocksModified != 0 || d.Stats.BlocksInserted != 0 || + d.Stats.BlocksDeleted != 0 { + b.Fatalf("a document compared with itself reported edits: %+v", d.Stats) + } + } +} + +// BenchmarkSegment is the parse half alone, so a regression can be attributed +// to segmentation or to alignment rather than to "the diff". +func BenchmarkSegment(b *testing.B) { + src := benchSpec(false) + + b.ReportAllocs() + b.SetBytes(int64(len(src))) + for b.Loop() { + if len(Segment(src)) == 0 { + b.Fatal("the fixture segmented into no blocks") + } + } +} + +// BenchmarkDiffWords is the inline word diff on its own: one modified paragraph +// against its edit, which is what runs once per modified block above. +func BenchmarkDiffWords(b *testing.B) { + oldText := strings.Repeat("A revision must carry an id, and that id is stable "+ + "across every revision of the document that carries it. ", 12) + newText := strings.Repeat("A revision must carry an identifier, and the id is "+ + "stable across each revision of the document carrying it. ", 12) + + b.ReportAllocs() + b.SetBytes(int64(len(oldText) + len(newText))) + for b.Loop() { + if len(DiffWords(oldText, newText)) == 0 { + b.Fatal("two different paragraphs produced no spans") + } + } +}