Skip to content

perf(serialize): id-level graph_to_turtle writer (#4898) - #6658

Merged
jeswr merged 5 commits into
mainfrom
perf/turtle-serializer
Oct 5, 2026
Merged

jeswr merged 5 commits into
mainfrom
perf/turtle-serializer

Conversation

@jeswr

@jeswr jeswr commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by Jesse · project thread

Summary

Closes #4898: graph_to_turtle was slower than Oxigraph 0.5's Turtle writer on a document-shaped graph.

Root cause. graph_to_turtle / graph_to_turtle_with called write_turtle(&graph_triples(g), ..). That first decoded every triple into owned oxrdf terms (three String allocations per row, plus the literal datatype). It then cloned each subject several times per row, rendered every predicate into a fresh String key and hashed it, and rendered every IRI twice (a header dry run into a probe string, then the body). On a document graph (many subjects, few predicates, long text literals), this per-row bookkeeping was most of the cost.

Fix (all in the feature-gated sparq-engine-serialize crate, behind serialize-rdf):

  • New id-level writer behind graph_to_turtle and graph_to_turtle_with:
    • IRI and blank-node renderings are cached per dictionary id.
    • Literals render from the borrowed Dict::term_parts record, with the datatype suffix cached.
    • Inline integers skip the Term rebuild.
    • The used-prefix set is collected during rendering, so the header needs no second pass.
    • Grouping is done on u32 ids. When the input is already grouped, as iter_ids (SPO) order is, no reordering happens.
    • Triple terms and directional language tags fall back to the generic renderer for that one object.
  • escape_string / escape_iri: copy unescaped runs in bulk instead of pushing per char.
  • write_prefix_header, which the generic and streaming paths use, now uses a compiled PrefixTable: no probe render, no subject Term clone.
  • New example turtle_vs_oxttl: a same-box comparison against oxttl 0.2's TurtleSerializer (the writer behind Oxigraph 0.5's RdfSerializer) on a deterministic document-shaped graph. Elements are headings, paragraphs and list items with varied-length text, order integers, links, and some @en labels.

Output.

  • Byte-identical to the previous writer on the benchmark corpus (cmp of before/after dumps, both default and custom prefixes).
  • New test id_writer_matches_generic_writer pins graph_to_turtle_with == write_turtle(&graph_triples(g), ..) and a parse round-trip. It covers inline and big integers, plain, typed, custom-datatype, language and directional literals, escape-heavy strings, blank nodes, triple terms, non-simple local names, and nested and equal-length namespaces, across four prefix maps.
  • turtle_row_order_groups_like_write_turtle_body covers the non-grouped ordering path.

Two behaviour fixes the new test found. Both are in the shared generic path, so the old and new writers still agree:

  1. write_iri tie-break bug. The guard compared the best match's local-part length against the candidate's namespace length. So it neither kept the longest namespace nor broke ties by label order, both of which the doc comment promises. It now follows the documented rule. Output changes only when two registered namespaces both match the same IRI with a simple local name. The coverage test that pinned the old arithmetic was rewritten as write_iri_longest_namespace_wins.
  2. RDF 1.2 base direction dropped. Turtle, both plain and pretty, emitted "x"@ar--rtl as "x"@ar, so a re-parse lost the direction. It now emits --rtl / --ltr.

Minor: a prefix label containing : (invalid Turtle) used to be mis-detected by the header's split_once(':') re-parse. The compiled table now marks the actual prefix used.

Measurements (non-canonical work-box measurements)

Shared 4-core dev box running other builds concurrently. Each number is the minimum of 200 to 300 iterations. These are signals, not canonical numbers; the canonical/EC2 bar from epic #2600 has not been run.

Corpus: turtle_vs_oxttl 1500 = 1,500 document elements, 8,396 triples, custom doc:/ont: prefixes.

writer before after
sparq graph_to_turtle_with 11.8 ms 1.55 ms
oxttl TurtleSerializer, triples pre-decoded 3.7–4.2 ms (same)
oxttl TurtleSerializer, including id → term decode 5.7–6.2 ms (same)

Reproduce: cargo run --release -p sparq-engine-serialize --features serialize-rdf --example turtle_vs_oxttl -- 1500 200.

On this synthetic shape the old writer was about 2–3× slower than oxttl, a bigger gap than the ~32% the reporter measured on their real graph; the new writer is about 2.4× faster than oxttl. I did not have the reporter's ruddydoc fixture, so their graph is not measured here.

The streaming writer (graph_to_turtle_streaming) still uses the generic path. Only its prefix header got faster. It stays byte-identical to graph_to_turtle, which the existing streamed_equals_buffered test checks.

Base gate (always required)

  • cargo build --workspace succeeds. (Not run: crate-scoped only on this shared box; left to CI.)
  • cargo clippy --workspace --exclude sparq-py --all-targets -- -D warnings is clean. (Ran cargo clippy -p sparq-engine-serialize --all-targets --all-features -- -D warnings: clean. Full workspace left to CI.)
  • The code this PR touches is formatted (matching the surrounding committed style).
  • cargo test passes for every crate this PR touches. (cargo test -p sparq-engine-serialize --features serialize-rdf,streaming-serialization: 123 lib tests and all integration tests pass, after merging current origin/main.)

Targeted re-evaluation (check the rows that apply to your change)

  • Public API: no new or changed pub item. I still updated skills/data-formats/SKILL.md to document the longest-namespace rule and the new example.
  • Wasm: the feature-off wasm bundle does not compile this crate (serialize-rdf is opt-in), so the byte-identity gate is unaffected. The scripts were not run.
  • Cargo dependencies: Cargo.lock gains only edges to crates already in the lock: rustc-hash 2.1.3 (optional, enabled only by serialize-rdf) and oxttl 0.2.3 (dev-dependency). No new crate versions. cargo deny / cargo audit are not installed on this box, so they were not run.

Ratchets and conventions

  • I did not lower any conformance / perf / coverage ratchet.
  • No hard-coded performance numbers added to markdown. The numbers appear only in this PR body.
  • No TODO/FIXME markers.
  • If this change makes a doc statement false, I updated that doc in the same change (write_iri comment, SKILL.md).

Security

  • This change does not introduce a security regression. It adds no unsafe (the crate keeps forbid(unsafe_code)). The datatype cache is keyed by the dictionary-owned &str address and length while the graph is borrowed immutably.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ScyGGohDhirnbLbrUSnA5n


Generated by Claude Code

claude added 2 commits October 5, 2026 19:13
graph_to_turtle / graph_to_turtle_with decoded every triple into owned oxrdf
terms, cloned subjects several times per row, rendered each predicate into a
fresh String key, and rendered every IRI twice (header dry run + body). The
new writer works on dictionary ids: IRI / blank-node renderings are cached per
id, literals render from borrowed term_parts records, inline integers skip the
Term rebuild, and the used-prefix set is collected while rendering. Output is
byte-identical to write_turtle(&graph_triples(g), ..), pinned by a new test.

Also:
- escape_string / escape_iri copy unescaped runs in bulk.
- write_prefix_header uses a compiled PrefixTable (no probe render / clone).
- write_iri tie-break compared the best LOCAL length to the candidate
  NAMESPACE length; it now implements the documented longest-namespace rule.
- Turtle (plain and pretty) dropped the RDF 1.2 base direction of
  directional language-tagged literals (@ar--rtl re-parsed as @ar).
- New example turtle_vs_oxttl: same-box comparison vs oxttl's
  TurtleSerializer on a document-shaped graph.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScyGGohDhirnbLbrUSnA5n
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScyGGohDhirnbLbrUSnA5n
@jeswr jeswr self-assigned this Oct 5, 2026
@jeswr
jeswr marked this pull request as ready for review October 5, 2026 19:34
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScyGGohDhirnbLbrUSnA5n
@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 0bd1b156ab25. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Longest-namespace selection can produce invalid Turtle — crates/sparq-engine-serialize/src/serialize.rs:244
    With prefixes a → http://ex/ns and z → http://ex/, serializing <http://ex/ns-foo> previously produced valid z:ns-foo; this change produces a:-foo. The shared is_simple_pn_local helper accepts leading - and ., although Turtle requires them to be escaped in that position. The new namespace selection therefore makes previously valid exports unparsable, affecting both the id writer and generic Turtle/TriG paths.

    Correct the shared helper’s first-character validation so these candidates fall back to a valid shorter prefix or full IRI. Add a parse-round-trip regression test using overlapping namespaces.

Verdict: Fix the prefix-selection regression before merging.

Turtle's PN_LOCAL must start with PN_CHARS_U | ':' | [0-9] | PLX, so an
unescaped leading '-' or '.' is invalid. is_simple_pn_local accepted both,
and with longest-namespace selection overlapping prefixes (a -> http://ex/ns,
z -> http://ex/) turned <http://ex/ns-foo> into the unparsable a:-foo.
The helper now rejects those first characters, so candidates fall back to
a shorter valid prefix (z:ns-foo) or the full IRI.

Adds a parse round-trip regression test with overlapping namespaces over
the id-level writer, the generic and pretty Turtle writers, and the
compact and pretty TriG writers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScyGGohDhirnbLbrUSnA5n
@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head c15f474709bb. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Longest-namespace selection can emit undeclared graph prefixes — crates/sparq-engine-serialize/src/serialize.rs:218
    With prefixes a → http://ex/ and b → http://ex/ns, serialize GRAPH <http://ex/nsLongEnoughLocalPart> { <http://ex/s> <http://ex/p> <http://ex/o> . }. The new rule renders the graph name as b:LongEnoughLocalPart, but TriG header collection examines only triples and therefore declares only a:. The output is invalid TriG. Before this change, this input rendered the graph name using the declared a: prefix. Buffered, streaming, and pretty TriG writers are affected.

    Include the names of emitted named graphs in used-prefix collection across all three TriG paths, retaining the corrected longest-namespace rule.

Verdict: Fix the TriG prefix regression before merging.

1 similar comment
@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head c15f474709bb. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

  1. [medium] Longest-namespace selection can emit undeclared graph prefixes — crates/sparq-engine-serialize/src/serialize.rs:218
    With prefixes a → http://ex/ and b → http://ex/ns, serialize GRAPH <http://ex/nsLongEnoughLocalPart> { <http://ex/s> <http://ex/p> <http://ex/o> . }. The new rule renders the graph name as b:LongEnoughLocalPart, but TriG header collection examines only triples and therefore declares only a:. The output is invalid TriG. Before this change, this input rendered the graph name using the declared a: prefix. Buffered, streaming, and pretty TriG writers are affected.

    Include the names of emitted named graphs in used-prefix collection across all three TriG paths, retaining the corrected longest-namespace rule.

Verdict: Fix the TriG prefix regression before merging.

…JSON-LD

The TriG `@prefix` header (buffered, streaming and pretty) and the compacted
JSON-LD `@context` were collected from the graphs' triples only, but the
writers also prefix-compact each named graph's NAME. With a: -> http://ex/
and b: -> http://ex/ns, GRAPH <http://ex/nsLongEnoughLocalPart> rendered as
`GRAPH b:LongEnoughLocalPart` under a header declaring only a:, which is
invalid TriG (and a JSON-LD graph @id whose prefix is missing from the
context re-expands to a different IRI).

Every used-prefix set now comes from one walk, `note_dataset_iris`, over
every IRI position a dataset writer may compact: subject / predicate /
object IRIs (through triple terms, including literal datatypes) and the
name of every emitted named graph. The longest-namespace rule is kept. The
TriG headers no longer clone the union of all graphs' triples, and the
pretty header uses the compiled PrefixTable instead of a probe render per
IRI.

Tests: trig_declares_prefixes_used_only_by_graph_names round-trips the
reviewer's dataset through all six TriG entry points (parse back, dataset
isomorphism); jsonld_context_declares_prefixes_used_only_by_graph_names.
The turtle_vs_oxttl example now also times the TriG writers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScyGGohDhirnbLbrUSnA5n
@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head f11d9314ce29. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

No correctness, security, soundness or design problems found.

Verdict: The PR appears safe to merge as is; Cargo tests were not rerun in this read-only review.

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

Local ci-fast gate (GitHub Actions outage; Jesse approved local-gate merges). PR head f11d9314 merged with main 09a50b09:

  • clippy -D warnings (core crates, all targets): pass
  • nextest (core crates, ci profile): pass
  • doctests (core crates): pass
  • W3C SPARQL conformance 1229/1229 (ratchet 1229): pass

Squash-merging under the local-gate rule.


Generated by Claude Code

@jeswr
jeswr merged commit 8c5676e into main Oct 5, 2026
9 checks passed
@jeswr
jeswr deleted the perf/turtle-serializer branch October 5, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: Turtle serialization (graph_to_turtle) ~32% slower than Oxigraph 0.5 baseline on a real-document-shaped graph

2 participants