diff --git a/AGENTS.md b/AGENTS.md index af77b51..8cdfd8d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1183,15 +1183,23 @@ Known weaknesses, so you neither trip over them nor assume they are intentional: **The fixed-size limits are the real boundary, not an oversight.** `sizeof(schema_t)` is 51 KB because every `field_def` carries `cases[16]` and `lookup[16]` unconditionally. Raising `SCHEMA_MAX_FIELDS` to fit mla20's 67 fields would put it past 110 KB, which - defeats the point of a firmware-tier interpreter. `repeat` is the last construct it has - no field type for, and at 3 schemas it is worth less than widening the harness to cover - `transform` (26) and `bitfield_string` (24). + defeats the point of a firmware-tier interpreter. Its largest *capability* gaps are + `transform` (26 schemas, and with it `polynomial`, `sqrt`, `pow`, `log`, `floor`, + `clamp`) and `bitfield_string`/`version_string` (24); `repeat` is the last construct with + no field type at all, at 3. This said the first two were worth "widening the harness to + cover", which was backwards - they are the interpreter's gaps, not the harness's, and + CR-2026-035 corrected it here, in SESSION-NOTES.md and in the harness's own skip + reasons, which had said "not built by the struct API" and were read as harness limits. Two things to keep straight when reading that report. **A skipped schema is not a passing one**, and **a harness limitation is not a C gap** - inline `match`, `byte_group`, - `object`, `$ref` and ports are named separately because C supports them and the harness - does not. Adding a construct to C without extending the harness would produce a feature - with no cross-check, which is why the TLV work is CR-2026-033 and not CR-2026-032. + `object` and `$ref` are named separately because C supports them and the harness does + not. Ports are the other way round and were listed here in error: the header says + outright that it has no port selection, so a port-based schema is a C gap. When in + doubt, grep the header before writing down which side a limit sits on - every + misattribution in this file and in SESSION-NOTES.md would have been caught by that. + Adding a construct to C without extending the harness would produce a feature with no + cross-check, which is why the TLV work is CR-2026-033 and not CR-2026-032. `src/test_comprehensive.c` is deliberately out of `make test-c`: 22 of its 160 assertions encode the pre-CR-2026-009 lookup and enum behaviour that PS-105/PS-269 changed, and its diff --git a/Makefile b/Makefile index 00e9758..a869336 100644 --- a/Makefile +++ b/Makefile @@ -63,7 +63,7 @@ PROTO_C = $(patsubst proto/%.proto,src/%.pb.c,$(PROTO_SRCS)) CXX = g++ CXXFLAGS = -std=c++17 -Wall -Wextra -O3 -Iinclude -.PHONY: all clean test test-c selftest validate-devices ci docs-index docs-index-check score-check validate-examples hypothesis coverage proto help codec benchmark generate-codec pytest pytest-cov coverage-html coverage-all validate fuzz fuzz-quick fuzz-hypothesis fuzz-go fuzz-c test-go test-java test-dotnet test-languages +.PHONY: all clean test test-c selftest validate-devices ci docs-index docs-index-check score-check validate-examples hypothesis coverage proto help codec benchmark bench-c generate-codec pytest pytest-cov coverage-html coverage-all validate fuzz fuzz-quick fuzz-hypothesis fuzz-go fuzz-c test-go test-java test-dotnet test-languages all: $(TEST_BIN) @@ -259,6 +259,14 @@ $(BENCHMARK_BIN): src/benchmark.cpp include/env_sensor_codec.h | $(BUILD_DIR) benchmark: $(BENCHMARK_BIN) $(BENCHMARK_BIN) +# Benchmarks the runtime interpreter in include/schema_interpreter.h against the Python +# reference on one corpus schema. Distinct from `benchmark` above, which times a small +# interpreter defined inline in src/benchmark.cpp and never includes that header - the +# figures in docs/SPEC-IMPLEMENTATION-STATUS.md come from this target, not that one. +# Not in `ci`: it is a measurement, and its numbers depend on the machine. +bench-c: $(VENV)/bin/activate + $(PYTHON) tools/benchmark-c-interpreter.py + # Generate C codec from schema (old single-file generator) generate-codec: python3 tools/generate-c.py examples/env_sensor.yaml -o include/env_sensor_codec.h diff --git a/SESSION-NOTES.md b/SESSION-NOTES.md index 5c5d5eb..d8e95df 100644 --- a/SESSION-NOTES.md +++ b/SESSION-NOTES.md @@ -23,11 +23,18 @@ Green as of the last run: Corpus 1229 -> 1250. Decode floors 1193 -> 1239. Encode round-trip: reference 1131 -> 1163, Go 1144 -> 1173, Java 1143 -> 1163, C# 1144 -> 1164. -**The next work on C is widening the harness, not the interpreter.** `tlv` (CR-2026-033) -and `flagged` (CR-2026-034) are done, and `repeat` is the last construct C has no field type -for - 3 schemas, worth less than the two harness limits above it: `transform` (26 schemas) -and `bitfield_string` (24). Whether C supports either is unknown *because the harness cannot -build them*, which is exactly the question the harness exists to answer. +**The next work on C is the interpreter, and the two biggest items are `transform` (26 +schemas) and `bitfield_string` (24).** CR-2026-035 corrected this paragraph, which said the +opposite: it called those two *harness* limits whose status was "unknown because the harness +cannot build them". They are the interpreter's own gaps, and the status was never unknown - +`grep -cw transform include/schema_interpreter.h` is 0, as are `polynomial`, `sqrt`, `pow`, +`log`, `floor`, `clamp`, `compute` and `version_string`. The harness cannot build them +because there is nothing in C to build them with. + +The misreading came from the skip reasons themselves, which said "not built by the struct +API" - true, but it reads as a limit on the harness, and I took it that way and wrote it +down. They now say "the interpreter has no transform pipeline". After `transform` and +`bitfield_string`, `repeat` is the last construct with no field type at all (3 schemas). Whatever is done next: **do not add a construct to C without extending `tools/c-corpus-harness.py` in the same change**, or the new field type lands uncovered - and diff --git a/docs/INDEX.md b/docs/INDEX.md index a31f659..eca008a 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -44,7 +44,7 @@ Generated inventory of this repository: what lives where, what each document cov | [`SCHEMA-DEVELOPMENT-GUIDE.md`](SCHEMA-DEVELOPMENT-GUIDE.md) | Best practices for creating complete, validated payload schemas. | 194 | | [`SCHEMA-LANGUAGE-REFERENCE.md`](SCHEMA-LANGUAGE-REFERENCE.md) | Complete reference for the LoRa Alliance Payload Schema specification (v0.5.0). | 1152 | | [`SESSION-NOTES-2026-02-25.md`](SESSION-NOTES-2026-02-25.md) | The prototype tests were using a custom REQ-xxx-yyy numbering scheme that was inconsistent with the... | 78 | -| [`SPEC-IMPLEMENTATION-STATUS.md`](SPEC-IMPLEMENTATION-STATUS.md) | Feature support matrix across reference implementations. | 439 | +| [`SPEC-IMPLEMENTATION-STATUS.md`](SPEC-IMPLEMENTATION-STATUS.md) | Feature support matrix across reference implementations. | 457 | | [`TTN-CODEC-CONVERSION-GUIDE.md`](TTN-CODEC-CONVERSION-GUIDE.md) | Complete guide for AI-assisted conversion of The Things Network device repository codecs to Payload... | 549 | | [`WOT-REFERENCE.md`](WOT-REFERENCE.md) | Reference for mapping LoRaWAN payload schema fields to W3C WoT Thing Descriptions and SAREF... | 316 | @@ -77,6 +77,7 @@ Generated inventory of this repository: what lives where, what each document cov | `tools/analyze_codec.js` | Load and execute codec in sandbox | | `tools/analyze_ttn_codec.py` | TTN Codec Analyzer | | `tools/batch_analyze_codecs.py` | Batch analyze TTN Device Repository codecs. | +| `tools/benchmark-c-interpreter.py` | Benchmark the C interpreter against the Python reference on one corpus schema. | | `tools/benchmark_all.py` | Comprehensive codec benchmark | | `tools/benchmark_codecs.js` | Default test configuration | | `tools/binary_schema.py` | Binary Schema Encoder/Decoder for OTA Schema Transfer | diff --git a/docs/SPEC-IMPLEMENTATION-STATUS.md b/docs/SPEC-IMPLEMENTATION-STATUS.md index 19fdcb5..2f5a619 100644 --- a/docs/SPEC-IMPLEMENTATION-STATUS.md +++ b/docs/SPEC-IMPLEMENTATION-STATUS.md @@ -79,12 +79,12 @@ ordinary-field path, disagreeing with its own enum path. | Feature | Python | Java | Go | C | JS | |---------|--------|------|-----|---|-----| -| `sqrt` | ✓ | ✓ | ✓ | ✓ | ✓ | -| `abs` | ✓ | ✓ | ✓ | ✓ | ✓ | -| `pow` | ✓ | ✓ | ✓ | ✓ | ✓ | -| `log` / `log10` | ✓ | - | ✓ | ✓ | ✓ | -| `floor` / `ceiling` | ✓ | - | ✓ | ✓ | ✓ | -| `clamp` | ✓ | - | ✓ | ✓ | ✓ | +| `sqrt` | ✓ | ✓ | ✓ | - | ✓ | +| `abs` | ✓ | ✓ | ✓ | - | ✓ | +| `pow` | ✓ | ✓ | ✓ | - | ✓ | +| `log` / `log10` | ✓ | - | ✓ | - | ✓ | +| `floor` / `ceiling` | ✓ | - | ✓ | - | ✓ | +| `clamp` | ✓ | - | ✓ | - | ✓ | | `round` | ✓ | - | ✓ | - | ✓ | ### Computed Fields @@ -93,7 +93,7 @@ ordinary-field path, disagreeing with its own enum path. |---------|--------|------|-----|---|-----| | `type: number` | ✓ | ✓ | ✓ | ✓ | ✓ | | `ref: $field` | ✓ | ✓ | ✓ | ✓ | ✓ | -| `polynomial` | ✓ | - | ✓ | ✓ | ✓ | +| `polynomial` | ✓ | - | ✓ | - | ✓ | | `compute: {op, a, b}` | ✓ | - | ✓ | - | ✓ | | `guard` conditions | ✓ | - | ✓ | - | ✓ | @@ -113,12 +113,12 @@ ordinary-field path, disagreeing with its own enum path. | Feature | Python | Java | Go | C | JS | |---------|--------|------|-----|---|-----| | `type: object` | ✓ | ✓ | ✓ | ✓ | ✓ | -| `type: repeat` (count) | ✓ | ✓ | ✓ | ✓ | ✓ | -| `repeat` (count_field) | ✓ | ✓ | ✓ | ✓ | ✓ | -| `repeat` (until: end) | ✓ | ✓ | ✓ | ✓ | ✓ | +| `type: repeat` (count) | ✓ | ✓ | ✓ | - | ✓ | +| `repeat` (count_field) | ✓ | ✓ | ✓ | - | ✓ | +| `repeat` (until: end) | ✓ | ✓ | ✓ | - | ✓ | | `definitions` / `use` | ✓ | - | ✓ | - | ✓ | -| `ports` (fPort routing) | ✓ | ✓ | ✓ | ✓ | ✓ | -| `var` (variables) | ✓ | ✓ | ✓ | - | ✓ | +| `ports` (fPort routing) | ✓ | ✓ | ✓ | - | ✓ | +| `var` (variables) | ✓ | ✓ | ✓ | ✓ | ✓ | ### Encodings @@ -233,12 +233,18 @@ but report no quality object; the interpreters and the generated JS agree on it, **Embedded-optimized** - no dynamic allocation required. -- Full decode support - Binary schema loading (no YAML) - Programmatic schema building -- 32M msg/s throughput -- Missing: complex computed fields, definitions +- 8.5M decodes/s on a 15-field `flagged` frame - see Performance Benchmarks below, + and regenerate with `make bench-c` rather than trusting this figure +- Decodes every construct the corpus needs except `repeat`, and has no `transform` + pipeline at all, so no `polynomial`, `sqrt`, `pow`, `log`, `floor`, `clamp` +- Also missing: computed fields, `definitions` / `use`, `ports` (fPort routing) +- Has `tlv` (CR-2026-033) and `flagged` (CR-2026-034); `unknown: raw` is not + representable because there is nowhere to put the captured bytes - No encode support (decode-only) +- Measured against the corpus by `tools/c-corpus-harness.py`: 488 of 1239 vectors are + in schemas the struct API can build, and all 488 decode exactly as the corpus expects ### JavaScript (`tools/generate_ts013_codec.py` output) @@ -317,20 +323,32 @@ Tested with DL-5TM schema (8 fields, flagged construct, polynomial transform). ### C Interpreter (AMD Ryzen 9 7950X3D) -**Not comparable to the DL-5TM rows above** — the C interpreter has no `flagged` or -`polynomial` support, so this uses a simpler 5-field frame (u8 protocol, u16 device -id, s16 temperature with `div`, u8 humidity with `div`, u16 battery). The Python -figure was measured on the same machine with the same schema and payload, so the two -rows here are comparable to each other and to nothing else in this document. +Regenerate with `make bench-c` (`tools/benchmark-c-interpreter.py`). Frame: +`schemas/devices/decentlab/dl-lid.yaml`, vector `vendor_reference_2` — a 29-byte +payload decoding to **15 fields**: three plain, then a two-group `flagged` covering +twelve more, two of them scaled with `div`. + +**Still not comparable to the DL-5TM rows above**, but for one reason now rather than +two. The C interpreter gained `flagged` in CR-2026-034, so the frame no longer avoids +it; what it has no support for at all is `transform` and `polynomial`, which DL-5TM +needs for two of its fields. Both rows below were measured on the same machine from +the same schema file and the same payload, so they are comparable to each other and to +nothing else in this document. | Implementation | Throughput | Latency | |----------------|------------|---------| -| C interpreter (`include/schema_interpreter.h`) | 20.5M ops/s | 0.05 µs | -| Python interpreter | 141K ops/s | 7.1 µs | -| **Ratio** | **145x** | | - -Stripped executable including the whole interpreter and the schema: **18.6 KB** -(`gcc -O2 -Os`, header-only so everything inlines). That size and throughput are why +| C interpreter (`include/schema_interpreter.h`) | 8.5M ops/s | 0.12 µs | +| Python interpreter (`tools/schema_interpreter.py`) | 40K ops/s | 25 µs | +| **Ratio** | **~210x** | | + +Both figures time `decode` alone, with the schema built once outside the loop, so +neither includes YAML parsing or schema construction. Across three runs C held +8.4–8.6M ops/s and Python 39–41K, so the ratio is good to about ±5%; the table is +rounded to match. The earlier numbers here (20.5M ops/s, 145x) were measured on a +5-field frame with no `flagged`, and are not comparable to these. + +Stripped executable including the whole interpreter and the schema: **18.2 KB** +(`cc -O2 -Os`, header-only so everything inlines). That size and throughput are why the C interpreter is a candidate for a full-featured embedded-Linux gateway decoder and not only for the MCU binary-schema path — see AGENTS.md for what it still lacks. diff --git a/tests/test_cr_2026_035_benchmark_flagged_claim.py b/tests/test_cr_2026_035_benchmark_flagged_claim.py new file mode 100644 index 0000000..fd859e1 --- /dev/null +++ b/tests/test_cr_2026_035_benchmark_flagged_claim.py @@ -0,0 +1,258 @@ +"""CR-2026-035: the stale `flagged` claim in the C benchmark, and what it was hiding. + +`docs/SPEC-IMPLEMENTATION-STATUS.md` justified a simplified C benchmark frame by saying +the interpreter "has no `flagged` or `polynomial` support". CR-2026-034 gave it `flagged`, +so half that sentence was false. Fixing the sentence turned up three larger problems. + +**The numbers were not reproducible.** No committed thing produced the 20.5M ops/s in that +table. `src/benchmark.cpp` looks like the source and is not: it times a small interpreter +defined inline in itself, and never includes `schema_interpreter.h`. A third figure, 32M +msg/s, sat in the same document's C entry. `tools/benchmark-c-interpreter.py` and +`make bench-c` now regenerate both rows, and the C schema is built from **the same YAML the +Python reference reads**, by reusing the corpus harness's `schema_source()` - a +hand-transcribed C copy can drift from the YAML, and then the two rows are not measuring +the same work. Measured: C 8.5M ops/s, Python 40K, ~210x, on a 15-field `flagged` frame. + +**Ten cells of the feature matrix were wrong about C.** It claimed `polynomial`, `sqrt`, +`abs`, `pow`, `log`, `floor`/`ceiling`, `clamp`, all three `repeat` rows and `ports` - each +of which is zero occurrences in the header - and denied `var`, which the interpreter has +had all along and which `flagged` depends on. The test below re-derives the C column from +the header instead of hardcoding the corrections. + +**And the harness's own wording had misled me.** Its skip reasons said `transform` and +`bitfield_string` were "not built by the struct API", which reads as a limit on the +harness. SESSION-NOTES.md and AGENTS.md both concluded from that "the next work on C is +widening the harness, not the interpreter", naming those two as harness limits whose status +was "unknown because the harness cannot build them". Backwards: they are the interpreter's +two largest gaps, 26 schemas and 24, and `grep -cw transform include/schema_interpreter.h` +was 0 the whole time. The reasons now say "the interpreter has no transform pipeline", and +both documents are corrected. +""" + +import json +import re +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +REPO_ROOT = Path(__file__).resolve().parent.parent +DOC = REPO_ROOT / "docs" / "SPEC-IMPLEMENTATION-STATUS.md" +HEADER = REPO_ROOT / "include" / "schema_interpreter.h" +BENCH = REPO_ROOT / "tools" / "benchmark-c-interpreter.py" +HARNESS = REPO_ROOT / "tools" / "c-corpus-harness.py" +NOTES = REPO_ROOT / "SESSION-NOTES.md" +AGENTS = REPO_ROOT / "AGENTS.md" +FRAME = REPO_ROOT / "schemas" / "devices" / "decentlab" / "dl-lid.yaml" + +C_COLUMN = 4 # | Feature | Python | Java | Go | C | JS | + + +def doc_rows(): + """{row label: [cells]} for every 6-column matrix row in the document.""" + rows = {} + for line in DOC.read_text().splitlines(): + if not line.startswith("|"): + continue + cells = [c.strip() for c in line.split("|")[1:-1]] + if len(cells) == 6: + rows.setdefault(cells[0], cells) + return rows + + +class TestTheStaleClaimIsGone: + def test_the_benchmark_no_longer_says_c_has_no_flagged(self): + text = DOC.read_text() + assert "the C interpreter has no `flagged`" not in text + assert "has no `flagged` or" not in text + + def test_it_still_explains_why_dl_5tm_is_out_of_reach(self): + """The frame is still simplified - for one reason now, not two.""" + section = DOC.read_text().split("### C Interpreter")[1].split("### Java")[0] + assert "transform" in section and "polynomial" in section + assert "CR-2026-034" in section, "the reason it changed should be traceable" + + def test_the_unreproducible_figures_are_gone_or_marked_historical(self): + text = DOC.read_text() + assert "32M msg/s" not in text, "a third, unsourced throughput figure" + # 20.5M may still appear, but only as an explicitly superseded number. + for match in re.finditer(r"20\.5M", text): + window = text[max(0, match.start() - 260):match.start() + 120] + assert "earlier numbers" in window or "not comparable" in window, window + + +class TestTheBenchmarkIsReproducible: + def test_the_tool_exists_and_is_executable(self): + assert BENCH.is_file() + assert BENCH.stat().st_mode & 0o111, "not executable" + + def test_the_doc_names_the_command_that_regenerates_it(self): + assert "make bench-c" in DOC.read_text() + + def test_there_is_a_make_target(self): + text = (REPO_ROOT / "Makefile").read_text() + assert re.search(r"^bench-c:", text, re.M) + assert re.search(r"^\.PHONY:.*\bbench-c\b", text, re.M) + + def test_it_is_not_in_ci(self): + """A measurement whose numbers depend on the machine does not belong in CI.""" + text = (REPO_ROOT / "Makefile").read_text() + ci = next(l for l in text.splitlines() if l.startswith("ci:")) + assert "bench-c" not in ci, ci + + def test_it_builds_the_c_schema_from_the_yaml_rather_than_a_copy(self): + """A transcribed C schema can drift; then the two rows measure different work.""" + text = BENCH.read_text() + assert "c-corpus-harness.py" in text + assert "schema_source" in text + + def test_the_old_cpp_benchmark_is_not_the_source_of_these_numbers(self): + """It never includes the header, which is why it could not have been.""" + cpp = (REPO_ROOT / "src" / "benchmark.cpp").read_text() + assert "schema_interpreter.h" not in cpp + assert "src/benchmark.cpp" in (REPO_ROOT / "Makefile").read_text() + + def test_the_loop_cannot_be_optimised_away(self): + """Without a consumer for the results, -O2 may delete the loop entirely.""" + text = BENCH.read_text() + assert "volatile" in text + assert "sink" in text + + @pytest.mark.slow + def test_it_runs_and_both_sides_decode_the_same_field_count(self, tmp_path): + out = tmp_path / "b.json" + done = subprocess.run( + [sys.executable, str(BENCH), "--iterations", "20000", + "--python-iterations", "300", "--rounds", "2", "--json", str(out)], + cwd=REPO_ROOT, capture_output=True, text=True) + assert done.returncode == 0, done.stdout + done.stderr + report = json.loads(out.read_text()) + assert report["flagged"] is True, "the frame must exercise flagged" + assert report["c_fields_decoded"] == report["python_fields_decoded"], report + assert report["c_fields_decoded"] >= 15, report + assert report["c"]["ops_per_sec"] > report["python"]["ops_per_sec"] + + +class TestTheFrameActuallyExercisesFlagged: + def test_the_default_frame_has_a_flagged_construct(self): + schema = yaml.safe_load(FRAME.read_text()) + assert any(isinstance(f, dict) and "flagged" in f for f in schema["fields"]) + + def test_it_has_no_transform_which_c_could_not_run(self): + text = FRAME.read_text() + assert "transform:" not in text and "polynomial:" not in text + + def test_the_doc_names_this_frame(self): + section = DOC.read_text().split("### C Interpreter")[1].split("### Java")[0] + assert "dl-lid" in section + + +class TestTheMatrixAgreesWithTheHeader: + """Re-derived from the header, so a future capability change breaks this, not prose.""" + + #: (matrix row label, keyword whose presence in the header means C supports it) + DERIVED = [ + ("`polynomial`", "polynomial"), + ("`sqrt`", "sqrt"), + ("`abs`", "abs"), + ("`pow`", "pow"), + ("`clamp`", "clamp"), + ("`type: repeat` (count)", "FIELD_TYPE_REPEAT"), + ("`repeat` (count_field)", "FIELD_TYPE_REPEAT"), + ("`repeat` (until: end)", "FIELD_TYPE_REPEAT"), + ("`type: object`", "FIELD_TYPE_OBJECT"), + ("`flagged`", "FIELD_TYPE_FLAGGED"), + ("`tlv`", "FIELD_TYPE_TLV"), + ] + + @pytest.mark.parametrize("label,keyword", DERIVED, ids=[d[0] for d in DERIVED]) + def test_the_c_cell_matches_the_header(self, label, keyword): + rows = doc_rows() + assert label in rows, f"no matrix row {label!r}; ids may have drifted" + supported = re.search(rf"\b{re.escape(keyword)}\b", HEADER.read_text()) is not None + cell = rows[label][C_COLUMN] + claims = cell == "✓" + assert claims == supported, ( + f"{label}: doc says C={cell!r} but the header " + f"{'has' if supported else 'does not have'} {keyword!r}" + ) + + def test_variables_are_credited_now(self): + """C has var_get/var_set/var_has, and `flagged` reads a mask through them.""" + assert "static inline bool var_has(" in HEADER.read_text() + assert doc_rows()["`var` (variables)"][C_COLUMN] == "✓" + + def test_ports_are_not_credited(self): + """The header says outright that it has no port selection.""" + assert "no port selection" in HEADER.read_text() + assert doc_rows()["`ports` (fPort routing)"][C_COLUMN] == "-" + + +class TestTheSkipReasonsNameTheRightSide: + """The wording that produced a backwards conclusion in two documents.""" + + @pytest.fixture(scope="class") + def report(self, tmp_path_factory): + out = tmp_path_factory.mktemp("c35") / "r.json" + done = subprocess.run([sys.executable, str(HARNESS), "--json", str(out)], + cwd=REPO_ROOT, capture_output=True, text=True) + assert done.returncode == 0, done.stdout[-2000:] + return json.loads(out.read_text()) + + def test_the_old_wording_is_gone(self): + """Checked on the reason strings, not the file. + + The file still contains the phrase - in the comment explaining why it was + wrong. Reading the source text flagged that comment, which is the fourth + loose-anchor slip of this kind recorded in SESSION-NOTES.md, so this imports + the table and inspects its values. + """ + import importlib.util + spec = importlib.util.spec_from_file_location("h35", HARNESS) + harness = importlib.util.module_from_spec(spec) + spec.loader.exec_module(harness) + + offenders = [(k, v) for k, v in harness.UNREACHABLE_KEYS.items() + if "not built by the struct API" in v] + assert offenders == [], offenders + # The one remaining "struct API" reason is explicit about being a C gap. + for key, reason in harness.UNREACHABLE_KEYS.items(): + if "struct API" in reason: + assert "C gap" in reason, (key, reason) + + def test_transform_is_reported_as_an_interpreter_gap(self, report): + reasons = [r for r in report["skips"] if "transform" in r] + assert reasons, sorted(report["skips"]) + assert all("interpreter" in r for r in reasons), reasons + + def test_it_is_the_largest_gap_and_that_is_visible(self, report): + transform = max(v for k, v in report["skips"].items() if "transform" in k) + assert transform >= 20, report["skips"] + + def test_the_measurement_did_not_regress(self, report): + assert report["failures"] == [], report["failures"][:6] + assert report["attempted"] >= 488, report["attempted"] + assert report["attempted"] + report["skipped_vectors"] == report["corpus_vectors"] + + +class TestTheDocumentsNoLongerSayTheOpposite: + def test_the_notes_do_not_call_transform_a_harness_limit(self): + text = NOTES.read_text() + assert "widening the harness, not the interpreter" not in text + assert "the interpreter, and the two biggest items" in text + + def test_the_notes_record_that_it_was_knowable_by_grep(self): + assert "grep -cw transform" in NOTES.read_text() + + def test_agents_no_longer_calls_them_harness_work(self): + text = AGENTS.read_text() + assert 'worth less than widening the harness to cover' not in text + assert "largest *capability* gaps" in text + + def test_agents_no_longer_lists_ports_as_supported(self): + """Normalised: the claim spans a wrapped line.""" + text = " ".join(AGENTS.read_text().split()) + assert "`$ref` and ports are named separately because C supports them" not in text + assert "Ports are the other way round" in text diff --git a/tools/benchmark-c-interpreter.py b/tools/benchmark-c-interpreter.py new file mode 100755 index 0000000..a45754b --- /dev/null +++ b/tools/benchmark-c-interpreter.py @@ -0,0 +1,276 @@ +#!/usr/bin/env python3 +"""Benchmark the C interpreter against the Python reference on one corpus schema. + +`docs/SPEC-IMPLEMENTATION-STATUS.md` carried a C row whose numbers no committed thing +produced. `src/benchmark.cpp` looks like the source but is not: it times a small +interpreter it defines inline plus a generated codec, and never includes +`schema_interpreter.h`. So the figures could not be checked, and the prose beside them +went stale without anything noticing - it justified a simplified benchmark frame by +saying the C interpreter "has no `flagged`", which CR-2026-034 made untrue. + +This regenerates both rows from source. The point of interest is that **the C schema is +built from the same YAML the Python reference reads**, by reusing +`c-corpus-harness.py`'s `schema_source()`. A hand-transcribed C copy of the schema is +the obvious way to write this and the wrong one: it can drift from the YAML silently, +and then the two rows are no longer measuring the same work. + +What is timed is `schema_decode` alone, with the schema built once outside the loop, so +the rows mean "decode a payload with a schema already in memory". Neither figure +includes YAML parsing or schema construction. + +The default frame is `dl-lid`, the richest `flagged` schema the struct API can build: +three plain fields and a two-group `flagged` covering twelve more. DL-5TM, which the +other rows in that document use, remains out of reach - it needs `polynomial` and +`transform`, and the C interpreter has neither. +""" + +import argparse +import importlib.util +import json +import shutil +import statistics +import subprocess +import sys +import tempfile +import time +from pathlib import Path + +import yaml + +REPO_ROOT = Path(__file__).resolve().parent.parent +HARNESS = REPO_ROOT / "tools" / "c-corpus-harness.py" +DEFAULT_SCHEMA = REPO_ROOT / "schemas" / "devices" / "decentlab" / "dl-lid.yaml" + +sys.path.insert(0, str(REPO_ROOT / "tools")) + + +def load_harness(): + """The corpus harness, imported for its schema builder rather than re-implemented.""" + spec = importlib.util.spec_from_file_location("c_corpus_harness", HARNESS) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +PROGRAM = '''/* Generated by tools/benchmark-c-interpreter.py - do not commit. + * + * Times schema_decode() with the schema built once, outside the loop. + */ +#include "schema_interpreter.h" +#include +#include +#include + +%(build)s + +/* The decoded values are accumulated into this and printed. Without a consumer the + * whole loop is dead code and -O2 is entitled to delete it, which would report an + * arbitrarily large ops/s. */ +static volatile long long sink = 0; + +static double seconds_for(int iterations, const uint8_t* p, int len, schema_t* s) { + decode_result_t r; + struct timespec a, b; + clock_gettime(CLOCK_MONOTONIC, &a); + for (int i = 0; i < iterations; i++) { + if (schema_decode(s, p, len, &r) != SCHEMA_OK) return -1.0; + sink += r.field_count; + } + clock_gettime(CLOCK_MONOTONIC, &b); + return (double)(b.tv_sec - a.tv_sec) + (double)(b.tv_nsec - a.tv_nsec) / 1e9; +} + +int main(void) { + static const uint8_t payload[] = {%(payload)s}; + const int len = %(len)d; + schema_t s; + build_0(&s); + + /* Prove it decodes before timing it. A benchmark of a failing decode is a + * benchmark of the error path. */ + decode_result_t once; + int rc = schema_decode(&s, payload, len, &once); + if (rc != SCHEMA_OK) { printf("ERR\\t%%d\\t%%s\\n", rc, once.error_msg); return 1; } + printf("FIELDS\\t%%d\\n", once.field_count); + + seconds_for(%(warmup)d, payload, len, &s); /* warm caches, ignore */ + + for (int round = 0; round < %(rounds)d; round++) { + double elapsed = seconds_for(%(iterations)d, payload, len, &s); + if (elapsed < 0) { printf("ERR\\tdecode failed mid-loop\\n"); return 1; } + printf("ROUND\\t%%.9f\\n", elapsed); + } + printf("SINK\\t%%lld\\n", sink); + return 0; +} +''' + + +def pick_vector(schema, wanted): + """The vector to time: the named one, else the longest payload. + + The longest exercises the most fields - for a `flagged` schema that means the most + groups set, which is the case worth timing. + """ + vectors = schema.get("test_vectors") or [] + if not vectors: + raise SystemExit("schema has no test_vectors to benchmark") + if wanted: + for vector in vectors: + if vector.get("name") == wanted: + return vector + raise SystemExit(f"no vector named {wanted!r}") + return max(vectors, key=lambda v: len(str(v.get("payload", "")))) + + +def build_c(schema, vector, args, harness): + source, reason = harness.schema_source(0, schema) + if source is None: + raise SystemExit(f"the struct API cannot build this schema: {reason}") + payload = bytes.fromhex(str(vector["payload"]).replace(" ", "")) + return PROGRAM % { + "build": "\n".join(source), + "payload": ", ".join(f"0x{b:02x}" for b in payload) or "0", + "len": len(payload), + "warmup": max(1000, args.iterations // 10), + "iterations": args.iterations, + "rounds": args.rounds, + }, payload + + +def run_c(program, args): + with tempfile.TemporaryDirectory() as tmp: + tmp = Path(tmp) + src, binary = tmp / "bench.c", tmp / "bench" + src.write_text(program) + compile_cmd = [args.cc, "-O2", "-Os", "-I", str(REPO_ROOT / "include"), + str(src), "-o", str(binary)] + done = subprocess.run(compile_cmd, capture_output=True, text=True) + if done.returncode != 0: + raise SystemExit(f"compile failed:\n{done.stderr[-3000:]}") + run = subprocess.run([str(binary)], capture_output=True, text=True) + if run.returncode != 0: + raise SystemExit(f"benchmark failed:\n{run.stdout}\n{run.stderr}") + + # Size is reported stripped, which is what an embedded target would ship. + size = None + if shutil.which("strip"): + stripped = tmp / "bench.stripped" + shutil.copy(binary, stripped) + if subprocess.run(["strip", str(stripped)], + capture_output=True).returncode == 0: + size = stripped.stat().st_size + + rounds, fields = [], None + for line in run.stdout.splitlines(): + kind, _, rest = line.partition("\t") + if kind == "ROUND": + rounds.append(float(rest)) + elif kind == "FIELDS": + fields = int(rest) + elif kind == "ERR": + raise SystemExit(f"C decode failed: {rest}") + if not rounds: + raise SystemExit(f"no timing rounds in output:\n{run.stdout}") + return rounds, fields, size + + +def run_python(schema, payload, args): + from schema_interpreter import SchemaInterpreter + + interpreter = SchemaInterpreter(schema) # built once, as on the C side + result = interpreter.decode(payload) + if getattr(result, "errors", None): + raise SystemExit(f"python decode failed: {result.errors}") + + rounds = [] + for _ in range(args.rounds): + # A fresh interpreter per round would time construction too; the C side builds + # once, so this does as well. + start = time.perf_counter() + for _ in range(args.python_iterations): + interpreter.decode(payload) + rounds.append(time.perf_counter() - start) + return rounds, len(result.data) + + +def best(rounds, iterations): + """The fastest round, not the mean. + + A benchmark's slow rounds are the ones the OS interfered with; the fastest is the + closest to the work itself. The spread is reported so a noisy machine is visible + rather than averaged into the number. + """ + per_op = [r / iterations for r in rounds] + fastest = min(per_op) + return { + "ops_per_sec": 1.0 / fastest, + "ns_per_op": fastest * 1e9, + "rounds": len(rounds), + "spread_pct": ((max(per_op) - fastest) / fastest * 100.0) if fastest else 0.0, + "median_ns_per_op": statistics.median(per_op) * 1e9, + } + + +def human(ops): + if ops >= 1e6: + return f"{ops / 1e6:.1f}M ops/s" + if ops >= 1e3: + return f"{ops / 1e3:.0f}K ops/s" + return f"{ops:.0f} ops/s" + + +def main(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--schema", type=Path, default=DEFAULT_SCHEMA) + parser.add_argument("--vector", help="test vector name (default: longest payload)") + parser.add_argument("--iterations", type=int, default=2_000_000, + help="C decodes per round (default 2000000)") + parser.add_argument("--python-iterations", type=int, default=20_000, + help="Python decodes per round (default 20000)") + parser.add_argument("--rounds", type=int, default=5) + parser.add_argument("--cc", default="cc") + parser.add_argument("--json", metavar="PATH") + args = parser.parse_args() + + schema = yaml.safe_load(args.schema.read_text()) + vector = pick_vector(schema, args.vector) + harness = load_harness() + + program, payload = build_c(schema, vector, args, harness) + c_rounds, c_fields, size = run_c(program, args) + py_rounds, py_fields = run_python(schema, payload, args) + + c = best(c_rounds, args.iterations) + py = best(py_rounds, args.python_iterations) + report = { + "schema": str(args.schema.relative_to(REPO_ROOT)), + "schema_name": schema.get("name"), + "vector": vector.get("name"), + "payload_bytes": len(payload), + "c_fields_decoded": c_fields, + "python_fields_decoded": py_fields, + "c": c, "python": py, + "ratio": c["ops_per_sec"] / py["ops_per_sec"] if py["ops_per_sec"] else None, + "stripped_bytes": size, + "flagged": any(isinstance(f, dict) and "flagged" in f + for f in schema.get("fields", [])), + } + + print(f"{report['schema']} vector={report['vector']} " + f"{report['payload_bytes']} B flagged={report['flagged']}") + print(f" fields decoded: C {c_fields}, Python {py_fields}") + print(f" C interpreter {human(c['ops_per_sec']):>12} " + f"{c['ns_per_op']:8.1f} ns/op (spread {c['spread_pct']:.1f}%)") + print(f" Python {human(py['ops_per_sec']):>12} " + f"{py['ns_per_op']:8.1f} ns/op (spread {py['spread_pct']:.1f}%)") + print(f" ratio {report['ratio']:.0f}x") + if size: + print(f" stripped {size / 1024:.1f} KB (interpreter + schema + harness)") + + if args.json: + Path(args.json).write_text(json.dumps(report, indent=2) + "\n") + + +if __name__ == "__main__": + main() diff --git a/tools/c-corpus-harness.py b/tools/c-corpus-harness.py index 6b97697..f467eaf 100644 --- a/tools/c-corpus-harness.py +++ b/tools/c-corpus-harness.py @@ -75,13 +75,25 @@ "object": "no nested object support in this harness", "$ref": "no $ref splicing in this harness", "name_from": "no name template in C (fixed-size name buffers)", - "transform": "transform chain not built by the struct API", - "polynomial": "computed field not built by the struct API", - "compute": "computed field not built by the struct API", - "ref": "computed field not built by the struct API", - "guard": "guard not built by the struct API", - "parts": "bitfield_string/version_string not built by the struct API", - "encoding": "sign_magnitude/bcd/gray not built by the struct API", + # These say "the interpreter has none" rather than "not built by the struct API", + # which was the wording here and read as a limit on this harness. It is not: the + # header has no transform machinery at all - `transform`, `polynomial`, `sqrt`, + # `pow`, `log`, `floor`, `clamp` and `compute` are each zero occurrences in + # include/schema_interpreter.h, as are `version_string`, `sign_magnitude`, `bcd` + # and `gray`. The struct API cannot build them because there is nothing to build. + # + # The distinction is the whole point of this harness and the old wording inverted + # it: SESSION-NOTES.md concluded "the next work on C is widening the harness, not + # the interpreter" and named `transform` (26 schemas) and `bitfield_string` (24) as + # harness limits whose status was "unknown". They are the interpreter's largest + # gaps and their status was knowable by grep. CR-2026-035 corrected both. + "transform": "the interpreter has no transform pipeline", + "polynomial": "the interpreter has no transform pipeline (polynomial)", + "compute": "the interpreter has no computed fields", + "ref": "the interpreter has no computed fields (ref)", + "guard": "the interpreter has no guard", + "parts": "the interpreter has no bitfield_string/version_string", + "encoding": "the interpreter has no sign_magnitude/bcd/gray encodings", "var": "variables are only read by match, which this harness skips", # AGENTS.md records this one: a `default` on a lookup or enum is Python, Go, Java and # C# only, because the struct has no slot for it. Building such a field without its