Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions docs/reference/application-benchmark-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,14 @@ the protocol or a summary, including a field the checker does not otherwise read
be finite and within ±(2⁵³ − 1), the interoperable JSON range. Any other number,
including the `NaN` and `Infinity` that Python's JSON parser accepts, is refused as
`protocol_number_out_of_range` or `summary_number_out_of_range`, alongside any code
for the field that holds it. A mean is the correctly rounded sum divided by the
count, so a mean at an exclusive ceiling gets the same verdict on every Python version.
for the field that holds it. A summary still gets the number code when
`summary_revision_mismatch` or `summary_workload_unknown` stops the rest of its check.
An integer of any length gets this code on every Python version, never an
unreadable-file code. No object may repeat a key, because the parser keeps only the
last value and the scan would never see the others: a repeated key is refused as
`protocol_key_duplicate` or `summary_key_duplicate` before any other check. A mean is
the correctly rounded sum divided by the count, so a mean at an exclusive ceiling gets
the same verdict on every Python version.

These are acceptance thresholds, not measured achievements.

Expand All @@ -320,8 +326,8 @@ record. It exits 1 when any verdict fails or is inconclusive. It exits 2, withou
judging, when the protocol or summary is invalid: for example a summary for another
cache class or scale point, a candidate marked `not_applicable`, a `not_applicable`
record that carries samples, a negative sample, a planned count that is not a positive
integer, a number outside ±(2⁵³ − 1), or a value of the wrong type. Diagnostics are closed codes and never repeat
input values.
integer, a number outside ±(2⁵³ − 1), a repeated object key, or a value of the wrong
type. Diagnostics are closed codes and never repeat input values.

A summary has this shape. `samples` lists only admitted samples; `flagged` and `failed`
count the planned samples left out, so `samples` plus both counts never exceeds
Expand Down
67 changes: 55 additions & 12 deletions scripts/check-application-benchmark-protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
workload pins, rotated lane order) are exact; budgets and sample counts are floors at
their frozen values, which a revision may tighten.
Every number anywhere in the protocol or a summary must be finite and lie within
MAX_JSON_NUMBER, including fields the checker does not otherwise read.
MAX_JSON_NUMBER, including fields the checker does not otherwise read, and no object
may repeat a key, so no value is hidden from that scan.
Diagnostics are closed codes from DIAGNOSTICS; they never echo input values or paths.
A budget verdict is one of VERDICTS. A statistic is judged only from complete,
sufficient, admitted samples: missing, short, flagged or unaccounted samples are
Expand Down Expand Up @@ -152,6 +153,11 @@
# integer range. Bounding every number keeps each statistic and ratio finite and keeps
# Python from converting an unbounded JSON integer to a float.
MAX_JSON_NUMBER = 2 ** 53 - 1
# Decimal digits in MAX_JSON_NUMBER. An integer literal with more digits is out of range
# whatever its value, so it is never converted exactly: Python 3.11 and later refuse to
# convert one of more than 4300 digits, which would turn this range error into an
# unreadable file on some interpreters and not others.
MAX_JSON_INTEGER_DIGITS = len(str(MAX_JSON_NUMBER))

DIAGNOSTICS = frozenset({
"protocol_unreadable",
Expand Down Expand Up @@ -199,9 +205,11 @@
"observer_policy_missing",
"known_limit_malformed",
"protocol_number_out_of_range",
"protocol_key_duplicate",
"summary_unreadable",
"summary_not_object",
"summary_number_out_of_range",
"summary_key_duplicate",
"summary_revision_mismatch",
"summary_workload_unknown",
"summary_cache_class_mismatch",
Expand Down Expand Up @@ -335,8 +343,15 @@ def _numbers_in_range(value):


def _same(value, frozen):
"""Exact JSON equality: unlike ==, 4.0 is not 4 and true is not 1."""
return json.dumps(value, sort_keys=True) == json.dumps(frozen, sort_keys=True)
"""Exact JSON equality: unlike ==, 4.0 is not 4 and true is not 1.

Every frozen value serializes, so a value that cannot (an integer beyond Python's
conversion limit, or a non-JSON object) is simply different, never an exception.
"""
try:
return json.dumps(value, sort_keys=True) == json.dumps(frozen, sort_keys=True)
except (TypeError, ValueError, RecursionError):
return False


def _count(value):
Expand Down Expand Up @@ -662,16 +677,18 @@ def validate_summary(protocol, summary):
"""
if not isinstance(summary, dict):
return ["summary_not_object"], None
# The range scan comes first, so an out-of-range number is reported even when the
# revision or workload stops the summary from being checked further.
found = set()
if not _numbers_in_range(summary):
found.add("summary_number_out_of_range")
if not _count(summary.get("protocol_revision")) or summary["protocol_revision"] != protocol["revision"]:
return ["summary_revision_mismatch"], None
return sorted(found | {"summary_revision_mismatch"}), None
workload_id = summary.get("workload")
workloads = {w["id"]: w for w in protocol["workloads"]}
workload = workloads.get(workload_id) if isinstance(workload_id, str) else None
if workload is None:
return ["summary_workload_unknown"], None
found = set()
if not _numbers_in_range(summary):
found.add("summary_number_out_of_range")
return sorted(found | {"summary_workload_unknown"}), None
# Budgets are frozen for the warm class only; other classes are reported separately.
if summary.get("cache_class") != protocol["cache_classes"]["budget_class"]:
found.add("summary_cache_class_mismatch")
Expand Down Expand Up @@ -828,9 +845,35 @@ def accepted(records):
return bool(records) and all(record["verdict"] in ACCEPTING_VERDICTS for record in records)


def _load(path, unreadable):
class _DuplicateKey(Exception):
"""An object repeated a key; json.loads would keep only the last value."""


def _unique_object(pairs):
obj = dict(pairs)
if len(obj) != len(pairs):
raise _DuplicateKey
return obj


def _bounded_int(literal):
"""Parse a JSON integer literal; one too long to be in range parses as just out of range.

The sign is kept, and the result is still an int, so every type check treats it as
it would treat the exact value, and the range scan refuses it on every interpreter.
"""
if len(literal.lstrip("-")) > MAX_JSON_INTEGER_DIGITS:
return -(MAX_JSON_NUMBER + 1) if literal.startswith("-") else MAX_JSON_NUMBER + 1
return int(literal)


def _load(path, unreadable, duplicate):
"""Parse a JSON file, or return a closed code: `duplicate` for a repeated object key."""
try:
return json.loads(Path(path).read_text()), None
text = Path(path).read_text()
return json.loads(text, object_pairs_hook=_unique_object, parse_int=_bounded_int), None
except _DuplicateKey:
return None, duplicate
except (OSError, ValueError, UnicodeDecodeError, RecursionError):
return None, unreadable

Expand All @@ -843,7 +886,7 @@ def main(argv=None):
mode.add_argument("--evaluate", metavar="SUMMARY")
args = parser.parse_args(argv)

protocol, problem = _load(args.protocol, "protocol_unreadable")
protocol, problem = _load(args.protocol, "protocol_unreadable", "protocol_key_duplicate")
diagnostics = [problem] if problem else validate_protocol(protocol)
if diagnostics:
print(json.dumps({"status": "invalid", "diagnostics": diagnostics}))
Expand All @@ -853,7 +896,7 @@ def main(argv=None):
"orders": protocol["schedule"]["orders"]}, indent=2))
return 0
if args.evaluate:
summary, problem = _load(args.evaluate, "summary_unreadable")
summary, problem = _load(args.evaluate, "summary_unreadable", "summary_key_duplicate")
diagnostics, records = ([problem], []) if problem else evaluate(protocol, summary)
if diagnostics:
print(json.dumps({"status": "invalid", "diagnostics": diagnostics}))
Expand Down
101 changes: 100 additions & 1 deletion tests/python/test_application_benchmark_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
protocol_check = importlib.util.module_from_spec(SPEC)
SPEC.loader.exec_module(protocol_check)

PROTOCOL = json.loads((ROOT / "scripts/application-benchmark-protocol.json").read_text())
PROTOCOL_TEXT = (ROOT / "scripts/application-benchmark-protocol.json").read_text()
PROTOCOL = json.loads(PROTOCOL_TEXT)
PROTOCOL_DOC = (ROOT / "docs/reference/application-benchmark-protocol.md").read_text()
PERFORMANCE_DOC = (ROOT / "docs/performance.md").read_text()
MARKER = "PRIVATE-MARKER-9f2c"
Expand Down Expand Up @@ -99,6 +100,14 @@ def evaluate_file(summary):
return run_cli(["--evaluate", str(path)])


def run_cli_text(text, flag):
"""Run the CLI on raw JSON text, as a protocol (`--protocol`) or a summary (`--evaluate`)."""
with tempfile.TemporaryDirectory() as directory:
path = Path(directory) / "input.json"
path.write_text(text)
return run_cli([flag, str(path)])


def node_paths(node, prefix=(), list_limit=None):
"""Yield the path of every node below `node`; `list_limit` caps indices per list."""
if isinstance(node, dict):
Expand Down Expand Up @@ -226,6 +235,20 @@ def test_documents_carry_every_frozen_artifact_and_budget(self):
self.assertIn(value, PERFORMANCE_DOC)
self.assertIn("application-benchmark-protocol.md", PERFORMANCE_DOC)

def test_document_states_the_number_bound_and_its_codes(self):
# The page writes the bound as ±(2ⁿ − 1); every such bound must be MAX_JSON_NUMBER.
superscripts = str.maketrans("⁰¹²³⁴⁵⁶⁷⁸⁹", "0123456789")
bounds = re.findall(r"±\(2([⁰¹²³⁴⁵⁶⁷⁸⁹]+) − 1\)", PROTOCOL_DOC)
self.assertTrue(bounds)
# A bound written any other way would escape the check above.
self.assertEqual(PROTOCOL_DOC.count("±("), len(bounds))
for exponent in bounds:
self.assertEqual(2 ** int(exponent.translate(superscripts)) - 1, protocol_check.MAX_JSON_NUMBER)
for code in ("protocol_number_out_of_range", "summary_number_out_of_range", "protocol_key_duplicate",
"summary_key_duplicate", "summary_revision_mismatch", "summary_workload_unknown"):
self.assertIn(code, protocol_check.DIAGNOSTICS)
self.assertIn(f"`{code}`", PROTOCOL_DOC)


def _recast_as_compose(item):
for field in ("version", "commit", "asset_sha256", "tag", "asset", "capacity"):
Expand Down Expand Up @@ -702,6 +725,82 @@ def test_a_deeply_nested_unread_field_is_scanned_without_raising(self):
self.assertEqual(protocol_check.validate_protocol(mutated(lambda d: d.update(notes=deep))),
["protocol_number_out_of_range"])

def test_an_out_of_range_number_is_reported_with_an_early_refusal(self):
# The revision and workload codes stop the check early; the number code still appears.
cases = {
"summary_revision_mismatch": lambda s: s.update(protocol_revision=2),
"summary_workload_unknown": lambda s: s.update(workload=MARKER),
}
for code, change in cases.items():
for value in (float("nan"), 2 ** 53, -HUGE):
with self.subTest(code=code, value=repr(value)):
summary = small_summary()
change(summary)
summary["notes"] = value
expected = sorted([code, "summary_number_out_of_range"])
self.assertEqual(protocol_check.evaluate(PROTOCOL, summary), (expected, []))
self.assertEqual(evaluate_file(summary), (2, {"status": "invalid", "diagnostics": expected}))
# The number can be the revision itself: both codes describe the same field.
self.assertEqual(self.refused({**small_summary(), "protocol_revision": 2 ** 53}),
["summary_number_out_of_range", "summary_revision_mismatch"])

def test_a_repeated_key_is_refused_so_no_number_is_shadowed(self):
# json.loads keeps the last value of a repeated key, so `NaN` here was never scanned.
frozen = PROTOCOL_TEXT.rstrip()
self.assertTrue(frozen.endswith("}"))
shadowed = frozen[:-1] + ', "notes": NaN, "notes": 1}'
self.assertEqual(run_cli_text(shadowed, "--protocol"),
(2, {"status": "invalid", "diagnostics": ["protocol_key_duplicate"]}))
self.assertEqual(run_cli_text(frozen[:-1] + ', "notes": 1}', "--protocol")[0], 0)
nested = frozen.replace('"id": "candidate",', '"id": "candidate", "id": "candidate",', 1)
self.assertNotEqual(nested, frozen)
self.assertEqual(run_cli_text(nested, "--protocol"),
(2, {"status": "invalid", "diagnostics": ["protocol_key_duplicate"]}))
summary = json.dumps(small_summary())
self.assertEqual(run_cli_text(summary, "--evaluate")[0], 0)
repeated = summary.replace('"cache_class": "warm"', '"cache_class": NaN, "cache_class": "warm"', 1)
self.assertNotEqual(repeated, summary)
self.assertEqual(run_cli_text(repeated, "--evaluate"),
(2, {"status": "invalid", "diagnostics": ["summary_key_duplicate"]}))

def test_integer_literals_of_any_length_get_the_range_code_on_every_interpreter(self):
# Python 3.11+ refuses to convert an integer of more than 4300 digits, which made a
# 5000-digit literal `protocol_unreadable` there and out of range on 3.9.
limit = protocol_check.MAX_JSON_NUMBER
frozen = PROTOCOL_TEXT.rstrip()[:-1]
for literal in ("9" * 5000, "-" + "9" * 5000, str(limit + 1), str(10 ** 16), str(-(10 ** 16))):
with self.subTest(digits=len(literal)):
self.assertEqual(run_cli_text(frozen + ', "notes": ' + literal + "}", "--protocol"),
(2, {"status": "invalid", "diagnostics": ["protocol_number_out_of_range"]}))
summary = json.dumps({**small_summary(), "notes": 0}).replace('"notes": 0', '"notes": ' + literal)
self.assertEqual(run_cli_text(summary, "--evaluate"),
(2, {"status": "invalid", "diagnostics": ["summary_number_out_of_range"]}))
for literal in (str(limit), str(-limit), "0", "-0"):
with self.subTest(literal=literal):
self.assertEqual(run_cli_text(frozen + ', "notes": ' + literal + "}", "--protocol")[0], 0)
# A long literal in a read field gets the same codes as an exact out-of-range value there.
floor = PROTOCOL_TEXT.replace('"p95_min_samples": 60', '"p95_min_samples": ' + "9" * 5000, 1)
self.assertNotEqual(floor, PROTOCOL_TEXT)
exact = protocol_check.validate_protocol(mutated(lambda d: d["samples"].update(p95_min_samples=10 ** 400)))
self.assertEqual(run_cli_text(floor, "--protocol"), (2, {"status": "invalid", "diagnostics": exact}))
self.assertEqual(protocol_check._bounded_int("9" * 5000), limit + 1)
self.assertEqual(protocol_check._bounded_int("-" + "9" * 5000), -limit - 1)
self.assertEqual(protocol_check._bounded_int(str(limit)), limit)

def test_an_integer_beyond_the_conversion_limit_never_raises_in_process(self):
# Exact identity checks serialize values; on Python 3.11+ a 5000-digit integer
# cannot be serialized, which must mean "different", not an exception.
enormous = 10 ** 5000
for path in (("host", "logical_cpus"), ("lanes", 0, "commit"), ("workloads", 0, "pin"),
("samples", "p95_min_samples"), ("notes",)):
with self.subTest(path=path):
found = protocol_check.validate_protocol(replace_at(PROTOCOL, path, enormous))
self.assertIn("protocol_number_out_of_range", found)
self.assertTrue(set(found) <= protocol_check.DIAGNOSTICS)
summary = small_summary()
summary["notes"] = enormous
self.assertEqual(self.refused(summary), ["summary_number_out_of_range"])

def test_an_overflowing_ratio_fails_and_the_report_stays_strict_json(self):
summary = small_summary()
summary["lanes"]["compose"]["warm_startup_seconds"] = entry([5e-324] * 10)
Expand Down
Loading