feat(sync): add startup phase diagnostics for issue #47 - #57
Merged
Conversation
Implement per-phase startup instrumentation gated by BACKSCROLL_STARTUP_DIAGNOSTICS=1 environment variable. Measures and reports elapsed time and metrics for five startup phases: 1. Discovery — time to discover input sources via reader registry 2. Metadata — time to inspect file metadata for prefilter eligibility (file count) 3. Hashing — time to compute SHA-256 hashes (file counts and bytes hashed) 4. Parsing — time to parse discovered files 5. Database — time to write to SQLite including template backfill Output is written to stderr, leaving stdout byte-identical for --json and --robot contracts. Diagnostics are off by default with zero overhead when disabled. Tests verify: - Diagnostics output appears when env var is set - Output is suppressed when env var is unset (default) - stdout is byte-identical with and without diagnostics for --json and --robot - Benchmark included to measure performance across startup phases Closes the acceptance criterion from issue #47 that shipped unmet in PR #54. Claude-Session: https://claude.ai/code/session_019LDXzStaKrArqJKvy4z3eF
PR #57 added startup diagnostics but included a speculative attribution of the 230ms unattributed time to "startup coordination, lock, schema inspection". Measurement proves this wrong: lock + coordination + schema inspection sum to <1ms on empty corpus, but the gap swings 10x between cold first run (234ms) and warm subsequent runs (21ms). Add explicit measurements for: - Lock Acquisition: time to acquire startup coordination lock (sub-millisecond) - Index Prepare: time to open database and inspect schema via compat.InspectIndex (typically 8-10ms) Report Unattributed time (if present) honestly as I/O, config load, and other OS overhead, noting its page-cache sensitivity and 10x variance between runs. Remove the false attribution that would misdirect optimization efforts. All phases now explicitly measured or accounted for. Unattributed remainder documented as page-cache dependent, matching observed variance in real corpus testing (0.6ms fixed overhead, ~540ms I/O and page-cache variance on 870-file corpus). Tests updated to verify presence of Lock Acquisition and Index Prepare phases. CI gate: 85.3% coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the one acceptance criterion of #47 that shipped unmet in PR #54: "A benchmark or diagnostic reports time and bytes processed by each startup phase."
PR #54 delivered the metadata prefilter but shipped with invented performance numbers, precisely because nobody could measure. This makes startup measurable.
Surface
Gated by
BACKSCROLL_STARTUP_DIAGNOSTICS=1, written to theprogress io.WritermaybeAutoSyncalready receives (stderr). Off by default: with the variable unset, stderr is 0 bytes and behavior is unchanged.No CLI flag was added — that would touch the CLI contract (early validation, robot/json shapes) for what is only a diagnostic. stdout is byte-identical with and without the variable, verified for
--jsonand--robotby tests that compare the captured output, not merely the exit code.Observed output — real 870-file corpus, 201 MB index
Post-sync, cold page cache:
Steady state, corpus unchanged:
What the first measurements already showed
An earlier revision of this PR bucketed the residue as
Other (startup coordination, lock, schema inspection). That attribution was measured and found false: with an empty corpus and empty database those costs total 0.6 ms, and the real driver is database file I/O and page-cache state.Lock AcquisitionandIndex Prepareare now measured directly (36–50 µs and ~2.3 ms), and the residue is labelledUnattributedwith no speculative cause.Verification
Observed
just ci:GitHub CI green on
0b0d2d1, confirmed against the branch head viagit ls-remoterather than a cached PR field.Tests
TestStartupDiagnosticsWithEnvVar— each phase appears when enabled.TestStartupDiagnosticsDisabledByDefault— no output when unset.TestStartupDiagnosticsDoesNotAffectJSONOutput/...RobotOutput— stdout compared directly, so a leak into the machine-readable payloads fails the test.Relates to #47.
https://claude.ai/code/session_019LDXzStaKrArqJKvy4z3eF