From 7a7c0b42774471605e01f0c4644c724a5944d9c2 Mon Sep 17 00:00:00 2001 From: Yuansheng Wang Date: Fri, 15 May 2026 17:18:09 +0000 Subject: [PATCH 1/4] docs: spec for decoder/document instance pooling (#6) --- .../2026-05-15-decoder-pooling-design.md | 391 ++++++++++++++++++ 1 file changed, 391 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-15-decoder-pooling-design.md diff --git a/docs/superpowers/specs/2026-05-15-decoder-pooling-design.md b/docs/superpowers/specs/2026-05-15-decoder-pooling-design.md new file mode 100644 index 0000000..cdf4b4c --- /dev/null +++ b/docs/superpowers/specs/2026-05-15-decoder-pooling-design.md @@ -0,0 +1,391 @@ +# Decoder/Document Instance Pooling — Design (v1) + +**Date:** 2026-05-15 +**Status:** Design approved, awaiting implementation plan +**Project:** `lua-quick-decode` +**Issue:** [#6](https://github.com/membphis/lua-quick-decode/issues/6) +**Related:** [2026-05-15-rust-quick-json-decode-design.md](./2026-05-15-rust-quick-json-decode-design.md) + +--- + +## 1. Purpose & Non-Goals + +### Purpose + +Today every `qd.parse(payload)` constructs a fresh `Document` with a fresh `indices: Vec` (reserved at `buf.len() / 6`), plus a fresh `scratch` buffer and `SkipCache`. For a 10 MB payload the `indices` reservation alone is ~1.7 MB — large enough to take the `mmap` allocation path on glibc, costing roughly 10–50 µs per parse plus the symmetric dealloc on drop. + +This design adds a reusable `Decoder` that owns those buffers across parses. A `decoder:parse(payload)` call truncates the buffers (preserving capacity) and re-fills them, eliminating per-parse allocation overhead in steady state. Expected wins by payload size: + +| size | est. speedup | +|---|---:| +| small (2 KB) | ~5–10% | +| 100 KB – 1 MB | ~5–15% | +| 10 MB | ~1–3% (alloc is a small fraction of 2.9 ms) | + +### Non-Goals + +- **No change to validation semantics.** Phase 1 still performs the same shallow structural scan; Phase 2 still lazily decodes. The bytes returned by every accessor must be byte-identical to the existing `qd.parse()` path. This is enforced by a cross-equivalence test (§7). +- **No concurrent docs per decoder.** Only one live `Document` per `Decoder` at a time. Earlier docs become invalid as soon as `parse()` is called again (and are detected — see §4). +- **No thread safety.** A `Decoder` is single-threaded, same constraint as `qjd_doc` today. +- **No streaming.** Each `parse()` still requires a contiguous `&[u8]`. + +--- + +## 2. Confirmed Decisions + +| Aspect | Decision | +|---|---| +| API style | Parallel — `qd.new_decoder()` added; existing `qd.parse()` unchanged | +| Lua surface | Two distinct objects: `decoder` and `doc` | +| Liveness | One live `doc` per `decoder` at a time | +| Stale-doc safety | Generation counter; stale access returns `QJD_STALE_DOC` (Lua: `nil`) | +| Lifecycle methods | `decoder:reset()` + `decoder:destroy()` + ffi.gc fallback | +| Parse-error recovery | `parse()` auto-truncates on entry; partial state cannot leak | +| Rust architecture | `Document` renamed to `Decoder`, becomes stateful; `qjd_doc` becomes a thin `{decoder, gen}` handle | +| Backward compat | `qjd_parse()` + `qjd_free()` + all `qjd_get_*` / cursor APIs unchanged at the C ABI | + +--- + +## 3. API Surface + +### 3.1 Lua + +```lua +local qd = require("quickdecode") + +-- One-shot (unchanged, backward compatible) +local doc = qd.parse(payload) +doc:get_str("body.model") + +-- Pooled (new) +local decoder = qd.new_decoder() +for _, payload in ipairs(payloads) do + local doc = decoder:parse(payload) + -- ...access doc / cursor... +end +decoder:reset() -- optional: shrink buffers to zero capacity +decoder:destroy() -- optional: early release; decoder is dead afterwards +``` + +The returned `doc` uses the same `Doc` metatable as today. Every existing accessor (`get_str`, `get_i64`, `get_f64`, `get_bool`, `is_null`, `typeof`, `len`, `open`) works without change. The only new failure mode surfaced through the existing accessors is `QJD_STALE_DOC`, which the wrapper translates to `nil` — the same convention as `QJD_NOT_FOUND`. Callers migrating from `qd.parse` need no `pcall` additions. + +### 3.2 C ABI + +New symbols added to `include/lua_quick_decode.h`: + +```c +typedef struct qjd_decoder qjd_decoder; + +qjd_decoder* qjd_decoder_new(void); +void qjd_decoder_free(qjd_decoder*); +void qjd_decoder_reset(qjd_decoder*); +void qjd_decoder_destroy(qjd_decoder*); + +qjd_doc* qjd_decoder_parse(qjd_decoder*, const uint8_t* buf, size_t len, int* err_out); +``` + +The returned `qjd_doc*` is the same opaque type as today. All existing `qjd_get_*`, `qjd_open`, and `qjd_cursor_*` functions accept it. The cursor struct gains a `gen` field by repurposing one of its `_reserved` slots — see §4. + +A new error code is added to `src/error.rs` and the header: + +```c +QJD_STALE_DOC = 9 // doc/cursor's generation no longer matches its decoder +``` + +`qjd_strerror(9)` returns `"stale document or cursor"`. + +### 3.3 Lua wrapper + +```lua +-- In lua/quickdecode.lua +local NOT_FOUND = 2 +local STALE_DOC = 9 + +local function check_err(rc) + if rc == 0 then return true end + if rc == NOT_FOUND or rc == STALE_DOC then return false end -- nil-return path + error("quickdecode: " .. ffi.string(C.qjd_strerror(rc))) +end + +local Decoder = {}; Decoder.__index = Decoder + +function _M.new_decoder() + local ptr = C.qjd_decoder_new() + if ptr == nil then error("quickdecode: decoder alloc failed") end + return setmetatable({ + _ptr = ffi.gc(ptr, C.qjd_decoder_free), + }, Decoder) +end + +function Decoder:parse(payload) + self._payload = payload -- pin against Lua GC + local doc_ptr = C.qjd_decoder_parse(self._ptr, payload, #payload, err_box) + if doc_ptr == nil then + error("quickdecode: " .. ffi.string(C.qjd_strerror(err_box[0]))) + end + return setmetatable({ + _ptr = ffi.gc(doc_ptr, C.qjd_free), + _decoder = self, -- transitive payload pin + }, Doc) +end + +function Decoder:reset() C.qjd_decoder_reset(self._ptr) end +function Decoder:destroy() C.qjd_decoder_destroy(self._ptr) end +``` + +The reference chain `doc → decoder → payload` ensures: while any doc is reachable, its decoder stays alive; while the decoder is alive, the *current* payload stays alive. The previous parse's payload is dropped from `_payload` on the next `parse()` and becomes GC-eligible — old docs that referenced it are already stale (see §4) and cannot dereference it. + +--- + +## 4. Rust Architecture + +### 4.1 `Decoder` struct + +Replaces today's `Document`: + +```rust +pub struct Decoder { + indices: Vec, + scratch: RefCell>, + skip: RefCell, + current_buf: Option<&'static [u8]>, + gen: u32, + state: DecoderState, +} + +enum DecoderState { + Ready, + Parsed, + Destroyed, +} +``` + +No `Errored` state. A failed `parse()` returns the decoder to `Ready` — the next `parse()` truncates before scanning, so partial indices/scratch can never be observed (and the gen has already bumped, so any leftover doc is stale). + +### 4.2 `parse()` flow + +```rust +impl Decoder { + pub fn parse(&mut self, input: &[u8]) -> Result { + if matches!(self.state, DecoderState::Destroyed) { + return Err(qjd_err::QJD_INVALID_ARG); + } + self.gen = self.gen.wrapping_add(1); // invalidate all prior docs/cursors + self.indices.truncate(0); + self.scratch.borrow_mut().truncate(0); + self.skip.borrow_mut().clear(); + + match crate::scan::scan(input, &mut self.indices) { + Ok(_) => {} + Err(_) => { + self.state = DecoderState::Ready; + self.current_buf = None; + return Err(qjd_err::QJD_PARSE_ERROR); + } + } + self.indices.push(u32::MAX); // sentinel + self.current_buf = Some(unsafe { std::mem::transmute(input) }); + self.state = DecoderState::Parsed; + Ok(DocHandle { gen: self.gen }) + } +} +``` + +### 4.3 FFI doc handle + +```rust +pub struct qjd_doc { + decoder: NonNull, + gen: u32, + owns_decoder: bool, +} +``` + +- `qjd_decoder_parse()` returns a doc with `owns_decoder = false`. `qjd_free` drops only the doc box. +- `qjd_parse()` (legacy path) internally `Box::new(Decoder::new())`, parses into it, wraps in a doc with `owns_decoder = true`. `qjd_free` additionally `Box::from_raw`s the private decoder. +- All `qjd_get_*` / `qjd_open` / cursor functions are oblivious to `owns_decoder`. + +### 4.4 Cursor + +The `qjd_cursor` C struct currently has two `_reserved` fields. One is repurposed: + +```c +typedef struct { + const qjd_doc* doc; + uint32_t idx_start; + uint32_t idx_end; + uint32_t gen; // was _reserved0 + uint32_t _reserved1; +} qjd_cursor; +``` + +`qjd_open` / `qjd_cursor_open` / `qjd_cursor_field` / `qjd_cursor_index` populate `gen` from the doc's current decoder gen. Every cursor accessor checks `cursor.gen == decoder.gen` before dereferencing. + +The Lua wrapper traffics `qjd_cursor` by value through `cur_box`; no Lua-side changes are required. + +### 4.5 Refactor mechanics + +- `git mv src/doc.rs src/decoder.rs` — preserves blame. +- Inside the renamed file: `Document` → `Decoder`, add the new fields, replace `Document::parse(buf)` with `Decoder::new()` + `Decoder::parse(&mut self, buf)`. +- `src/cursor.rs`, `src/decode/*`, `src/scan/*` callers: function signatures change `&Document` → `&Decoder`. Logic untouched. +- `src/ffi.rs`: extract a small `check_doc_alive` helper used by every public entry point. The helper checks **state first** (`Destroyed` → `QJD_INVALID_ARG`) then **gen** (mismatch → `QJD_STALE_DOC`); this ordering matches §5.4. Add the four new `qjd_decoder_*` exports, each wrapped in `ffi_catch!` per the existing convention. +- `src/skip_cache.rs`: add `pub(crate) fn clear(&mut self)` (drops all entries but keeps slot 0) and `pub(crate) fn clear_and_shrink(&mut self)` (also calls `shrink_to_fit` on the inner `Vec` and `FxHashMap`). Both are needed by §5. + +--- + +## 5. Lifecycle Semantics + +### 5.1 `parse()` after `parse()` + +Generation bumps first thing in the new `parse()`. All prior docs and cursors become stale. Buffers truncate but keep capacity. After a successful parse, the new doc holds the new gen. + +### 5.2 `parse()` after parse error + +State is `Ready`, gen has already advanced, buffers are partially written but unreadable (all prior docs are stale, no doc was returned for the failed parse). The next `parse()` truncates again and proceeds. + +### 5.3 `reset()` + +```rust +pub fn reset(&mut self) { + self.gen = self.gen.wrapping_add(1); + self.indices = Vec::new(); // returns memory to allocator + self.scratch.borrow_mut().shrink_to(0); + self.skip.borrow_mut().clear_and_shrink(); + self.current_buf = None; + self.state = DecoderState::Ready; +} +``` + +Use case: just processed a one-off huge payload, don't want the decoder to keep that capacity around for the worker's lifetime. + +### 5.4 `destroy()` + +```rust +pub fn destroy(&mut self) { + self.gen = self.gen.wrapping_add(1); + let _ = std::mem::take(&mut self.indices); + let _ = std::mem::take(&mut *self.scratch.borrow_mut()); + self.skip.borrow_mut().clear_and_shrink(); + self.current_buf = None; + self.state = DecoderState::Destroyed; +} +``` + +After `destroy()`, every subsequent FFI entry returns `QJD_INVALID_ARG`. The decoder's own memory is reclaimed only when ffi.gc fires on `qjd_decoder_free` — `destroy()` just shaves off the bulk allocations early. + +### 5.5 Gen overflow + +`wrapping_add(1)`. At 1 ms/parse the counter wraps after ~50 days of *continuous* `parse()` calls on the same decoder. By that point any old doc reference is long collected by Lua GC. The theoretical risk is documented but not engineered against in v1 — listed in `README.md` under _Roadmap / Deferred_. + +--- + +## 6. Error Handling + +| Code | Name | When | Lua wrapper | +|------|------|------|-------------| +| 0 | OK | success | true | +| 1 | PARSE_ERROR | scan failed | raises | +| 2 | NOT_FOUND | path missing | nil | +| 3 | TYPE_MISMATCH | wrong type at path | raises | +| 4 | OUT_OF_RANGE | numeric overflow | raises | +| 5 | DECODE_FAILED | lazy decode failed | raises | +| 6 | INVALID_PATH | path syntax | raises | +| 7 | INVALID_ARG | null arg / destroyed decoder | raises | +| 8 | OOM | panic caught by `ffi_catch!` | raises | +| **9** | **STALE_DOC** | **gen mismatch** | **nil** | + +The `QJD_STALE_DOC` code value, `qjd_strerror` entry, and `lua/quickdecode.lua` mirror must all be kept in sync (per the existing convention noted in CLAUDE.md). + +--- + +## 7. Testing & Validation + +### 7.1 Rust unit tests (`src/decoder.rs::tests`) + +- `parse_then_parse_bumps_gen` — two successive parses, second doc's gen ≠ first's +- `parse_error_returns_to_ready` — malformed input leaves state at `Ready` and gen bumped +- `reset_shrinks_capacity` — large parse + reset → `indices.capacity() == 0` +- `destroy_sets_terminal_state` — post-destroy `parse()` / `reset()` return `QJD_INVALID_ARG` +- `gen_wraps_at_u32_max` — set gen near `u32::MAX`, confirm wrap behavior + +### 7.2 Rust integration tests + +New file `tests/decoder_ffi.rs`: + +- `decoder_doc_equivalence` — for every fixture under `benches/fixtures/`, parse via both `qjd_parse` and `qjd_decoder_parse`, run the same battery of accessors, assert byte-identical results. This is the load-bearing guarantee that validation semantics are unchanged. +- `stale_doc_returns_error` — parse → hold doc → parse again → call `qjd_get_str` on the old doc, expect `QJD_STALE_DOC` +- `stale_cursor_returns_error` — same but the stale entity is a cursor opened from the first doc +- `reset_invalidates_cursors` — parse → open cursor → `reset()` → cursor access returns `QJD_STALE_DOC` +- `destroyed_decoder_rejects_all_ops` — destroy → parse/reset/get_str all return `QJD_INVALID_ARG` + +### 7.3 Lua busted tests (`tests/lua/spec/decoder_spec.lua`) + +- `new_decoder` returns a usable object +- `parse` returns a `Doc` with all existing methods +- multiple successive parses on the same decoder return correct results +- stale doc access returns `nil`, not an error +- `reset` and `destroy` work; post-destroy ops raise (because the FFI returns `QJD_INVALID_ARG`, not the nil-coded `QJD_STALE_DOC`) + +### 7.4 Allocation counting + +A new test-only Cargo feature `count-allocs` installs a counting `GlobalAlloc` in `tests/alloc_count.rs`: + +```rust +#[global_allocator] +static ALLOC: CountingAlloc = CountingAlloc::new(); + +#[test] +fn pooled_path_amortizes_allocations() { + let mut decoder = Decoder::new(); + for _ in 0..3 { decoder.parse(PAYLOAD).unwrap(); } // warmup + let baseline = ALLOC.count(); + for _ in 0..1_000 { decoder.parse(PAYLOAD).unwrap(); } + let delta = ALLOC.count() - baseline; + assert!(delta < 50, "expected ≈0 allocs, got {}", delta); +} + +#[test] +fn fresh_decoder_per_parse_allocates() { + let baseline = ALLOC.count(); + for _ in 0..1_000 { + let mut d = Decoder::new(); // mimics the cost of the legacy qjd_parse path + d.parse(PAYLOAD).unwrap(); + } + assert!(ALLOC.count() - baseline > 1_000); +} +``` + +The feature gates the global allocator swap so it doesn't interfere with other tests. Run target: + +```sh +cargo test --release --features count-allocs --test alloc_count +``` + +### 7.5 Bench harness (`benches/lua_bench.lua`) + +Adds two cases: + +- `decoder:parse` reuse loop (new) +- `lua-cjson` per-iter (existing baseline, kept) + +Output is `wall_ms` (3-run median) per fixture per case. Existing `qd.parse` case stays as the baseline to measure improvement against. + +### 7.6 CI + +`.github/workflows/ci.yml` gains a fourth Rust matrix point: + +4. `cargo test --release --features count-allocs --test alloc_count` + +The three existing gates (default features, `--no-default-features`, `--features test-panic`) and the Lua busted job continue to run unchanged. The cross-equivalence test (§7.2) runs under gate 1 and gate 2 — catches any scanner-vs-decoder divergence as a side effect. + +### 7.7 Lint baseline + +`make lint`'s current 22 `missing_safety_doc` warnings on `qjd_*` exports will grow by ~5 (one per new `qjd_decoder_*` symbol). This is consistent with the existing README _Roadmap / Deferred_ entry; the bump will be noted there rather than treated as a regression. + +--- + +## 8. Deferred + +- Per-symbol safety docs on FFI exports (existing deferred item; grows by ~5 entries) +- Gen counter wrap protection (50-day continuous-use horizon; not engineered against in v1) +- Implicit module-level shared decoder (`qd.parse` keeping pooled buffers transparently) — possible future optimization once the explicit decoder API stabilizes From a1f53634f40a5253f1e2bd9c57cb3c65979a7bb5 Mon Sep 17 00:00:00 2001 From: Yuansheng Wang Date: Fri, 15 May 2026 17:27:37 +0000 Subject: [PATCH 2/4] feat(decoder): introduce reusable Decoder + qjd_decoder_* FFI surface (#6) WIP: in-progress implementation. Header / Lua wrapper / tests still pending. - Rename Document<'a> to Decoder (no lifetime param); add state machine (Ready / Parsed / Destroyed), generation counter, in-place parse() that truncates and re-fills indices / scratch / skip cache. - Add Decoder::reset() (shrink) and Decoder::destroy() (terminal). - Repurpose qjd_doc as a thin {decoder, gen, owns_decoder} handle. All existing qjd_get_* / qjd_open / cursor APIs keep working unchanged. - Add qjd_decoder_new / free / parse / reset / destroy exports. - Add check_doc_alive helper: Destroyed -> QJD_INVALID_ARG; gen mismatch -> QJD_STALE_DOC (new error code, value 9). - SkipCache gains clear() and clear_and_shrink() for reset / destroy paths. Builds clean (one dead-code warning on parse_oneshot to be addressed when qjd_parse is refactored to use it). --- src/cursor.rs | 22 ++-- src/decoder.rs | 291 ++++++++++++++++++++++++++++++++++++++++++++++ src/doc.rs | 125 -------------------- src/error.rs | 4 +- src/ffi.rs | 251 ++++++++++++++++++++++++++++++++++----- src/lib.rs | 2 +- src/skip_cache.rs | 14 +++ 7 files changed, 541 insertions(+), 168 deletions(-) create mode 100644 src/decoder.rs delete mode 100644 src/doc.rs diff --git a/src/cursor.rs b/src/cursor.rs index cc21556..dd64973 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -1,4 +1,4 @@ -use crate::doc::Document; +use crate::decoder::Decoder; use crate::error::qjd_err; use crate::path::{PathIter, PathSeg}; @@ -13,7 +13,7 @@ pub(crate) struct Cursor { } impl Cursor { - pub(crate) fn root(doc: &Document) -> Self { + pub(crate) fn root(doc: &Decoder) -> Self { // Find the closing index of the outermost container. // indices has a u32::MAX sentinel at the end. let n = doc.indices.len() as u32; @@ -21,7 +21,7 @@ impl Cursor { Cursor { idx_start: 0, idx_end: n - 2 } } - pub(crate) fn resolve(self, doc: &Document, path: &[u8]) -> Result { + pub(crate) fn resolve(self, doc: &Decoder, path: &[u8]) -> Result { let mut cur = self; for seg in PathIter::new(path) { let seg = seg?; @@ -31,7 +31,7 @@ impl Cursor { } } -fn step(doc: &Document, cur: Cursor, seg: &PathSeg) -> Result { +fn step(doc: &Decoder, cur: Cursor, seg: &PathSeg) -> Result { // The cursor must point at a container. let opener_byte = container_opener_byte(doc, cur) .ok_or(qjd_err::QJD_TYPE_MISMATCH)?; @@ -45,7 +45,7 @@ fn step(doc: &Document, cur: Cursor, seg: &PathSeg) -> Result { /// If `cur` points at a container, return its opener byte (`{` or `[`). /// Returns None for scalars. -fn container_opener_byte(doc: &Document, cur: Cursor) -> Option { +fn container_opener_byte(doc: &Decoder, cur: Cursor) -> Option { if cur.idx_start as usize >= doc.indices.len() { return None; } let pos = doc.indices[cur.idx_start as usize] as usize; let b = *doc.buf.get(pos)?; @@ -55,7 +55,7 @@ fn container_opener_byte(doc: &Document, cur: Cursor) -> Option { /// Iterate children of the container at `cur` and return a Cursor for the /// matching child. Populates the skip cache on the first visit; uses it on /// subsequent visits. -fn walk_children(doc: &Document, cur: Cursor, seg: &PathSeg) -> Result { +fn walk_children(doc: &Decoder, cur: Cursor, seg: &PathSeg) -> Result { let is_obj = matches!(seg, PathSeg::Key(_)); let mut cache = doc.skip.borrow_mut(); let (slot_n, was_cached) = cache.get_or_insert(cur.idx_start); @@ -123,7 +123,7 @@ fn walk_children(doc: &Document, cur: Cursor, seg: &PathSeg) -> Result Result { for (k, (&i, &cursor_end)) in starts.iter().zip(ends.iter()).enumerate() { let matched = if is_obj { @@ -155,7 +155,7 @@ fn resolve_in_known_children( /// - container: index after the matching closer (= closer_idx + 1) /// - string: index after the close '"' (= start + 2) /// - scalar: start itself (indices[start] IS the separator/closer) -pub(crate) fn find_value_span(doc: &Document, start: u32) -> Result<(u32, u32), qjd_err> { +pub(crate) fn find_value_span(doc: &Decoder, start: u32) -> Result<(u32, u32), qjd_err> { let pos = doc.indices[start as usize] as usize; let b = *doc.buf.get(pos).ok_or(qjd_err::QJD_PARSE_ERROR)?; match b { @@ -202,11 +202,11 @@ pub(crate) fn find_value_span(doc: &Document, start: u32) -> Result<(u32, u32), } } -pub(crate) fn resolve_single_key(doc: &Document, cur: Cursor, key: &[u8]) -> Result { +pub(crate) fn resolve_single_key(doc: &Decoder, cur: Cursor, key: &[u8]) -> Result { step(doc, cur, &PathSeg::Key(key)) } -pub(crate) fn resolve_single_idx(doc: &Document, cur: Cursor, idx: u32) -> Result { +pub(crate) fn resolve_single_idx(doc: &Decoder, cur: Cursor, idx: u32) -> Result { step(doc, cur, &PathSeg::Idx(idx)) } @@ -214,7 +214,7 @@ pub(crate) fn resolve_single_idx(doc: &Document, cur: Cursor, idx: u32) -> Resul mod tests { use super::*; - fn doc_of(s: &[u8]) -> Document<'_> { Document::parse(s).unwrap() } + fn doc_of(s: &[u8]) -> Decoder { Decoder::parse_oneshot(s).unwrap() } #[test] fn root_path_returns_root() { diff --git a/src/decoder.rs b/src/decoder.rs new file mode 100644 index 0000000..1da29bb --- /dev/null +++ b/src/decoder.rs @@ -0,0 +1,291 @@ +use std::cell::RefCell; + +use crate::error::qjd_err; +use crate::skip_cache::SkipCache; + +/// Lifecycle state of a [`Decoder`]. +/// +/// `Ready` — freshly constructed or just reset; no document parsed. +/// `Parsed` — last parse() succeeded; indices/scratch reflect `buf`. +/// `Destroyed` — destroy() has been called; only free() is valid. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum DecoderState { + Ready, + Parsed, + Destroyed, +} + +/// Reusable JSON decoder. Owns the structural-offset buffer, the lazy-decode +/// scratch buffer, and the Phase-2 skip cache, all of which are reused across +/// successive parses to avoid per-call allocation traffic. +/// +/// `buf` is stored with a `'static` lifetime: the caller (FFI / test helper) +/// is responsible for keeping the underlying bytes alive until the next +/// `parse()`, `reset()`, `destroy()`, or drop of the decoder. +pub struct Decoder { + pub(crate) indices: Vec, + pub(crate) scratch: RefCell>, + pub(crate) skip: RefCell, + /// Active input buffer. Empty slice (`&[]`) when state ≠ Parsed. + pub(crate) buf: &'static [u8], + /// Bumped on every parse(), reset(), and destroy() so prior docs/cursors + /// can detect that they reference stale state. + pub(crate) gen: u32, + pub(crate) state: DecoderState, +} + +impl Decoder { + pub fn new() -> Self { + Self { + indices: Vec::new(), + scratch: RefCell::new(Vec::new()), + skip: RefCell::new(SkipCache::new()), + buf: &[], + gen: 0, + state: DecoderState::Ready, + } + } + + /// Parse `input` into this decoder, replacing any previous parse. + /// + /// On success, `state` becomes `Parsed` and `gen` advances by one. + /// On parse error, `state` is `Ready`, `gen` has still advanced (so all + /// prior docs/cursors are stale), and the buffers are conceptually empty + /// (their capacity is retained for reuse). + /// + /// The decoder borrows `input` for the duration of its `Parsed` state. + /// The caller must ensure `input` is not freed before the next `parse()`, + /// `reset()`, `destroy()`, or drop. + pub fn parse(&mut self, input: &[u8]) -> Result<(), qjd_err> { + if matches!(self.state, DecoderState::Destroyed) { + return Err(qjd_err::QJD_INVALID_ARG); + } + self.gen = self.gen.wrapping_add(1); + self.indices.truncate(0); + self.scratch.borrow_mut().truncate(0); + self.skip.borrow_mut().clear(); + self.buf = &[]; + self.state = DecoderState::Ready; + + crate::scan::scan(input, &mut self.indices) + .map_err(|_| qjd_err::QJD_PARSE_ERROR)?; + self.indices.push(u32::MAX); + // SAFETY: caller upholds the lifetime contract above. + self.buf = unsafe { std::mem::transmute::<&[u8], &'static [u8]>(input) }; + self.state = DecoderState::Parsed; + Ok(()) + } + + /// Drop all cached state and release allocated capacity back to the + /// allocator. After `reset()`, the decoder is reusable. + /// + /// Bumps `gen` so any outstanding docs/cursors become stale. + pub fn reset(&mut self) { + if matches!(self.state, DecoderState::Destroyed) { return; } + self.gen = self.gen.wrapping_add(1); + self.indices = Vec::new(); + *self.scratch.borrow_mut() = Vec::new(); + self.skip.borrow_mut().clear_and_shrink(); + self.buf = &[]; + self.state = DecoderState::Ready; + } + + /// Permanently retire this decoder. Frees the bulk buffers immediately; + /// the [`Decoder`] struct itself is reclaimed when its owning Box is + /// dropped (typically via the FFI free function). + pub fn destroy(&mut self) { + if matches!(self.state, DecoderState::Destroyed) { return; } + self.gen = self.gen.wrapping_add(1); + let _ = std::mem::take(&mut self.indices); + let _ = std::mem::take(&mut *self.scratch.borrow_mut()); + self.skip.borrow_mut().clear_and_shrink(); + self.buf = &[]; + self.state = DecoderState::Destroyed; + } + + /// Convenience for tests and the legacy `qjd_parse` path: construct a + /// decoder and parse `input` in one call. + pub fn parse_oneshot(input: &[u8]) -> Result { + let mut d = Self::new(); + d.parse(input)?; + Ok(d) + } +} + +impl Default for Decoder { + fn default() -> Self { Self::new() } +} + +use crate::cursor::{Cursor, find_value_span}; +use crate::error::qjd_type; + +impl Decoder { + /// Inspect a cursor and return its JSON value type. + pub(crate) fn type_of(&self, cur: Cursor) -> Result { + let pos = *self.indices.get(cur.idx_start as usize) + .ok_or(qjd_err::QJD_PARSE_ERROR)? as usize; + let lead = self.buf.get(pos).copied().ok_or(qjd_err::QJD_PARSE_ERROR)?; + match lead { + b'"' => Ok(qjd_type::QJD_T_STR), + b'{' => Ok(qjd_type::QJD_T_OBJ), + b'[' => Ok(qjd_type::QJD_T_ARR), + _ => { + // For a scalar value the cursor's idx_start points at the + // structural char AFTER the scalar; the scalar's first byte + // lives between the previous structural char and this one. + let scalar_start = self.find_scalar_start(cur.idx_start)?; + match self.buf.get(scalar_start).copied() { + Some(b't') | Some(b'f') => Ok(qjd_type::QJD_T_BOOL), + Some(b'n') => Ok(qjd_type::QJD_T_NULL), + Some(b'-') | Some(b'0'..=b'9') => Ok(qjd_type::QJD_T_NUM), + _ => Err(qjd_err::QJD_PARSE_ERROR), + } + } + } + } + + /// Find the byte position of the first non-whitespace byte after the + /// structural character at `indices[idx - 1]`. Used to locate the first + /// byte of a scalar value. + pub(crate) fn find_scalar_start(&self, idx: u32) -> Result { + if idx == 0 { return Err(qjd_err::QJD_PARSE_ERROR); } + let prev = self.indices[(idx - 1) as usize] as usize; + let mut p = prev + 1; + while p < self.buf.len() && matches!(self.buf[p], b' '|b'\t'|b'\n'|b'\r') { + p += 1; + } + Ok(p) + } + + /// Count direct children of the container at `cur`. + /// Returns QJD_TYPE_MISMATCH for non-container cursors. + pub(crate) fn cursor_len(&self, cur: Cursor) -> Result { + let pos = self.indices[cur.idx_start as usize] as usize; + let b = *self.buf.get(pos).ok_or(qjd_err::QJD_PARSE_ERROR)?; + if b != b'{' && b != b'[' { + return Err(qjd_err::QJD_TYPE_MISMATCH); + } + let is_obj = b == b'{'; + // Empty container detection: byte after opener (skipping whitespace) + // is the closer position itself, meaning no value sits between them. + let closer_pos = self.indices[cur.idx_end as usize] as usize; + let mut p = pos + 1; + while p < closer_pos && matches!(self.buf[p], b' '|b'\t'|b'\n'|b'\r') { + p += 1; + } + if p == closer_pos { + return Ok(0); + } + let mut count: usize = 0; + let mut i = cur.idx_start + 1; + let end = cur.idx_end; + loop { + count += 1; + let value_idx_start = if is_obj { i + 3 } else { i }; + let (_cursor_end, skip_end) = find_value_span(self, value_idx_start)?; + let after_pos = self.indices[skip_end as usize] as usize; + if after_pos >= self.buf.len() { return Err(qjd_err::QJD_PARSE_ERROR); } + match self.buf[after_pos] { + b',' => { + i = skip_end + 1; + if i > end { return Err(qjd_err::QJD_PARSE_ERROR); } + } + b'}' | b']' => break, + _ => return Err(qjd_err::QJD_PARSE_ERROR), + } + } + Ok(count) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_simple_object() { + let d = Decoder::parse_oneshot(b"{\"a\":1}").unwrap(); + assert!(d.indices.len() >= 5); + assert_eq!(*d.indices.last().unwrap(), u32::MAX); + assert_eq!(d.state, DecoderState::Parsed); + assert_eq!(d.gen, 1); + } + + #[test] + fn parse_error_on_malformed() { + assert!(Decoder::parse_oneshot(b"{").is_err()); + } + + #[test] + fn parse_then_parse_bumps_gen() { + let mut d = Decoder::new(); + d.parse(b"{\"a\":1}").unwrap(); + let g1 = d.gen; + d.parse(b"[1,2,3]").unwrap(); + assert_eq!(d.gen, g1.wrapping_add(1)); + } + + #[test] + fn parse_error_returns_to_ready_and_bumps_gen() { + let mut d = Decoder::new(); + d.parse(b"{\"a\":1}").unwrap(); + let g1 = d.gen; + assert_eq!(d.state, DecoderState::Parsed); + + assert!(d.parse(b"{").is_err()); + assert_eq!(d.state, DecoderState::Ready); + assert_eq!(d.gen, g1.wrapping_add(1)); + assert!(d.buf.is_empty()); + } + + #[test] + fn reset_shrinks_capacity() { + let mut d = Decoder::new(); + let payload: Vec = { + let mut s = Vec::from(&b"{\"k\":["[..]); + for i in 0..100 { if i > 0 { s.push(b','); } s.extend_from_slice(b"1"); } + s.extend_from_slice(b"]}"); + s + }; + d.parse(&payload).unwrap(); + assert!(d.indices.capacity() > 0); + + d.reset(); + assert_eq!(d.state, DecoderState::Ready); + assert_eq!(d.indices.capacity(), 0); + assert!(d.buf.is_empty()); + } + + #[test] + fn destroy_sets_terminal_state() { + let mut d = Decoder::new(); + d.parse(b"{\"a\":1}").unwrap(); + d.destroy(); + assert_eq!(d.state, DecoderState::Destroyed); + + assert_eq!(d.parse(b"{\"b\":2}").unwrap_err(), qjd_err::QJD_INVALID_ARG); + d.reset(); + assert_eq!(d.state, DecoderState::Destroyed); + } + + #[test] + fn destroy_is_idempotent() { + let mut d = Decoder::new(); + d.parse(b"{\"a\":1}").unwrap(); + let g = d.gen; + d.destroy(); + let g_after = d.gen; + d.destroy(); + assert_eq!(d.gen, g_after, "second destroy must not bump gen"); + assert_ne!(g_after, g); + } + + #[test] + fn gen_wrap_does_not_panic() { + let mut d = Decoder::new(); + d.gen = u32::MAX - 1; + d.parse(b"{\"a\":1}").unwrap(); // gen = MAX + d.parse(b"{\"b\":2}").unwrap(); // wraps to 0 + assert_eq!(d.gen, 0); + } +} diff --git a/src/doc.rs b/src/doc.rs deleted file mode 100644 index 7c6ecda..0000000 --- a/src/doc.rs +++ /dev/null @@ -1,125 +0,0 @@ -use std::cell::RefCell; - -use crate::error::qjd_err; -use crate::skip_cache::SkipCache; - -pub struct Document<'a> { - pub(crate) buf: &'a [u8], - pub(crate) indices: Vec, - pub(crate) scratch: RefCell>, - pub(crate) skip: RefCell, -} - -impl<'a> Document<'a> { - pub fn parse(buf: &'a [u8]) -> Result { - let mut indices = Vec::new(); - crate::scan::scan(buf, &mut indices).map_err(|_| qjd_err::QJD_PARSE_ERROR)?; - // Sentinel simplifies boundary checks during Phase 2. - indices.push(u32::MAX); - Ok(Self { - buf, - indices, - scratch: RefCell::new(Vec::new()), - skip: RefCell::new(SkipCache::new()), - }) - } -} - -use crate::cursor::{Cursor, find_value_span}; -use crate::error::qjd_type; - -impl<'a> Document<'a> { - /// Inspect a cursor and return its JSON value type. - pub(crate) fn type_of(&self, cur: Cursor) -> Result { - let pos = *self.indices.get(cur.idx_start as usize) - .ok_or(qjd_err::QJD_PARSE_ERROR)? as usize; - let lead = self.buf.get(pos).copied().ok_or(qjd_err::QJD_PARSE_ERROR)?; - match lead { - b'"' => Ok(qjd_type::QJD_T_STR), - b'{' => Ok(qjd_type::QJD_T_OBJ), - b'[' => Ok(qjd_type::QJD_T_ARR), - _ => { - // For a scalar value the cursor's idx_start points at the - // structural char AFTER the scalar; the scalar's first byte - // lives between the previous structural char and this one. - let scalar_start = self.find_scalar_start(cur.idx_start)?; - match self.buf.get(scalar_start).copied() { - Some(b't') | Some(b'f') => Ok(qjd_type::QJD_T_BOOL), - Some(b'n') => Ok(qjd_type::QJD_T_NULL), - Some(b'-') | Some(b'0'..=b'9') => Ok(qjd_type::QJD_T_NUM), - _ => Err(qjd_err::QJD_PARSE_ERROR), - } - } - } - } - - /// Find the byte position of the first non-whitespace byte after the - /// structural character at `indices[idx - 1]`. Used to locate the first - /// byte of a scalar value. - pub(crate) fn find_scalar_start(&self, idx: u32) -> Result { - if idx == 0 { return Err(qjd_err::QJD_PARSE_ERROR); } - let prev = self.indices[(idx - 1) as usize] as usize; - let mut p = prev + 1; - while p < self.buf.len() && matches!(self.buf[p], b' '|b'\t'|b'\n'|b'\r') { - p += 1; - } - Ok(p) - } - - /// Count direct children of the container at `cur`. - /// Returns QJD_TYPE_MISMATCH for non-container cursors. - pub(crate) fn cursor_len(&self, cur: Cursor) -> Result { - let pos = self.indices[cur.idx_start as usize] as usize; - let b = *self.buf.get(pos).ok_or(qjd_err::QJD_PARSE_ERROR)?; - if b != b'{' && b != b'[' { - return Err(qjd_err::QJD_TYPE_MISMATCH); - } - let is_obj = b == b'{'; - // Empty container detection: byte after opener (skipping whitespace) - // is the closer position itself, meaning no value sits between them. - let closer_pos = self.indices[cur.idx_end as usize] as usize; - let mut p = pos + 1; - while p < closer_pos && matches!(self.buf[p], b' '|b'\t'|b'\n'|b'\r') { - p += 1; - } - if p == closer_pos { - return Ok(0); - } - let mut count: usize = 0; - let mut i = cur.idx_start + 1; - let end = cur.idx_end; - loop { - count += 1; - let value_idx_start = if is_obj { i + 3 } else { i }; - let (_cursor_end, skip_end) = find_value_span(self, value_idx_start)?; - let after_pos = self.indices[skip_end as usize] as usize; - if after_pos >= self.buf.len() { return Err(qjd_err::QJD_PARSE_ERROR); } - match self.buf[after_pos] { - b',' => { - i = skip_end + 1; - if i > end { return Err(qjd_err::QJD_PARSE_ERROR); } - } - b'}' | b']' => break, - _ => return Err(qjd_err::QJD_PARSE_ERROR), - } - } - Ok(count) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn parses_simple_object() { - let doc = Document::parse(b"{\"a\":1}").unwrap(); - assert!(doc.indices.len() >= 5); - assert_eq!(*doc.indices.last().unwrap(), u32::MAX); - } - - #[test] - fn parse_error_on_malformed() { - assert!(Document::parse(b"{").is_err()); - } -} diff --git a/src/error.rs b/src/error.rs index 270ea10..51c45e0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -12,6 +12,7 @@ pub enum qjd_err { QJD_INVALID_PATH = 6, QJD_INVALID_ARG = 7, QJD_OOM = 8, + QJD_STALE_DOC = 9, } #[repr(C)] @@ -36,6 +37,7 @@ pub fn strerror(code: qjd_err) -> &'static str { qjd_err::QJD_INVALID_PATH => "invalid path syntax", qjd_err::QJD_INVALID_ARG => "invalid argument", qjd_err::QJD_OOM => "out of memory", + qjd_err::QJD_STALE_DOC => "stale document or cursor", } } @@ -49,7 +51,7 @@ mod tests { qjd_err::QJD_OK, qjd_err::QJD_PARSE_ERROR, qjd_err::QJD_NOT_FOUND, qjd_err::QJD_TYPE_MISMATCH, qjd_err::QJD_OUT_OF_RANGE, qjd_err::QJD_DECODE_FAILED, qjd_err::QJD_INVALID_PATH, - qjd_err::QJD_INVALID_ARG, qjd_err::QJD_OOM, + qjd_err::QJD_INVALID_ARG, qjd_err::QJD_OOM, qjd_err::QJD_STALE_DOC, ] { assert!(!strerror(code).is_empty()); } diff --git a/src/ffi.rs b/src/ffi.rs index 09d4094..84d2206 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -6,9 +6,17 @@ //! Most exports share the same FFI obligations on the caller: //! //! - A `*mut qjd_doc` argument must be NULL or a pointer previously returned -//! by [`qjd_parse`] that has not yet been passed to [`qjd_free`]. +//! by [`qjd_parse`] or [`qjd_decoder_parse`] that has not yet been passed +//! to [`qjd_free`]. +//! - A `*mut qjd_decoder` argument must be NULL or a pointer previously +//! returned by [`qjd_decoder_new`] that has not yet been passed to +//! [`qjd_decoder_free`]. //! - The input buffer passed to [`qjd_parse`] must remain valid and -//! unmodified for as long as the document is alive; the document borrows it. +//! unmodified for the lifetime of the returned document. +//! - The input buffer passed to [`qjd_decoder_parse`] must remain valid and +//! unmodified until the next [`qjd_decoder_parse`] / [`qjd_decoder_reset`] +//! / [`qjd_decoder_destroy`] / [`qjd_decoder_free`] call on the same +//! decoder, or any doc operation referencing it. //! - Path / key pointer arguments must point to the indicated number of //! readable bytes, or be NULL when the length is `0`. //! - Out pointers must be non-NULL and writable for their target type when @@ -20,15 +28,20 @@ //! - A pointer/length pair returned by any `*_get_str` is invalidated by //! the next `*_get_str` call on the same document (scratch buffer reuse). //! +//! After [`qjd_decoder_parse`] is called on a decoder, all docs and cursors +//! produced by *prior* parses on that decoder become stale; operations on +//! them return [`qjd_err::QJD_STALE_DOC`] (Lua wrapper: `nil`). After +//! [`qjd_decoder_destroy`], all operations return `QJD_INVALID_ARG`. +//! //! Every export catches Rust panics at the FFI boundary and converts them //! to `QJD_OOM`; a panic must not be allowed to unwind across the boundary. #![allow(non_camel_case_types)] use std::os::raw::{c_char, c_int}; -use std::ptr; +use std::ptr::{self, NonNull}; -use crate::doc::Document; +use crate::decoder::{Decoder, DecoderState}; use crate::error::qjd_err; macro_rules! ffi_catch { @@ -41,8 +54,43 @@ macro_rules! ffi_catch { }}; } -/// Opaque type exported to C as `qjd_doc*`. -pub struct qjd_doc(pub(crate) Document<'static>); +// ── Opaque types ──────────────────────────────────────────────────────────── + +/// Opaque type exported to C as `qjd_decoder*`. +pub struct qjd_decoder(pub(crate) Decoder); + +/// Opaque type exported to C as `qjd_doc*`. A doc is a thin handle: +/// a pointer to the owning decoder plus the generation that was current +/// at the time the doc was produced. Successive [`qjd_decoder_parse`] (or +/// `reset` / `destroy`) calls bump the decoder's generation, so prior docs +/// detect they are stale via the gen check at every entry point. +pub struct qjd_doc { + decoder: NonNull, + gen: u32, + /// True for the one-shot [`qjd_parse`] path: the decoder is owned by + /// this doc and is freed when the doc is freed. False for docs produced + /// by [`qjd_decoder_parse`] (the user owns the decoder). + owns_decoder: bool, +} + +// ── Entry-point safety helpers ────────────────────────────────────────────── + +/// Validate `doc` and return the live decoder. Order matters: a destroyed +/// decoder is reported as `QJD_INVALID_ARG`, not `QJD_STALE_DOC`. +unsafe fn check_doc_alive(doc: *mut qjd_doc) -> Result<&'static Decoder, qjd_err> { + if doc.is_null() { return Err(qjd_err::QJD_INVALID_ARG); } + let d = &*doc; + let dec: &Decoder = &d.decoder.as_ref().0; + if matches!(dec.state, DecoderState::Destroyed) { + return Err(qjd_err::QJD_INVALID_ARG); + } + if dec.gen != d.gen { + return Err(qjd_err::QJD_STALE_DOC); + } + Ok(std::mem::transmute::<&Decoder, &'static Decoder>(dec)) +} + +// ── strerror ──────────────────────────────────────────────────────────────── /// Return a static NUL-terminated message for the given error code. /// @@ -53,7 +101,6 @@ pub struct qjd_doc(pub(crate) Document<'static>); /// and must not be freed. #[no_mangle] pub unsafe extern "C" fn qjd_strerror(code: c_int) -> *const c_char { - // Hardcoded NUL-terminated map; avoids runtime allocation and lifetime issues. let s: &'static [u8] = match code { 0 => b"ok\0", 1 => b"JSON parse error\0", @@ -64,12 +111,19 @@ pub unsafe extern "C" fn qjd_strerror(code: c_int) -> *const c_char { 6 => b"invalid path syntax\0", 7 => b"invalid argument\0", 8 => b"out of memory\0", + 9 => b"stale document or cursor\0", _ => b"unknown error code\0", }; s.as_ptr() as *const c_char } -/// Parse a JSON buffer into a document (Phase 1: structural scan). +// ── qjd_parse / qjd_free (one-shot, backward-compatible) ──────────────────── + +/// Parse a JSON buffer into a one-shot document (Phase 1: structural scan). +/// +/// Internally allocates a private decoder owned by the returned document. +/// For repeated parses on hot paths, prefer [`qjd_decoder_new`] + +/// [`qjd_decoder_parse`]. /// /// # Safety /// @@ -78,7 +132,7 @@ pub unsafe extern "C" fn qjd_strerror(code: c_int) -> *const c_char { /// - `err_out` must point to a writable `int`, or be NULL (in which case the /// function returns NULL with no error code written). /// - The buffer must remain valid and unmodified for the lifetime of the -/// returned `qjd_doc*`; the document borrows it. +/// returned `qjd_doc*`; the underlying decoder borrows it. /// - On success, the returned pointer must be freed exactly once with /// [`qjd_free`]. #[no_mangle] @@ -92,13 +146,20 @@ pub unsafe extern "C" fn qjd_parse( if !err_out.is_null() { *err_out = qjd_err::QJD_INVALID_ARG as c_int; } return ptr::null_mut(); } - let slice: &'static [u8] = std::slice::from_raw_parts(buf, len); - match Document::parse(slice) { - Ok(d) => { + let dec_ptr = Box::into_raw(Box::new(qjd_decoder(Decoder::new()))); + let slice: &[u8] = std::slice::from_raw_parts(buf, len); + match (*dec_ptr).0.parse(slice) { + Ok(()) => { *err_out = qjd_err::QJD_OK as c_int; - Box::into_raw(Box::new(qjd_doc(d))) + let doc = qjd_doc { + decoder: NonNull::new_unchecked(dec_ptr), + gen: (*dec_ptr).0.gen, + owns_decoder: true, + }; + Box::into_raw(Box::new(doc)) } Err(e) => { + let _ = Box::from_raw(dec_ptr); *err_out = e as c_int; ptr::null_mut() } @@ -113,38 +174,170 @@ pub unsafe extern "C" fn qjd_parse( } } -/// Free a document returned by [`qjd_parse`]. NULL is a no-op. +/// Free a document returned by [`qjd_parse`] or [`qjd_decoder_parse`]. +/// NULL is a no-op. For docs produced by [`qjd_parse`], this also frees the +/// private decoder. For docs produced by [`qjd_decoder_parse`], the decoder +/// is left alone — free it with [`qjd_decoder_free`]. /// /// # Safety /// -/// `doc` must be NULL or a pointer previously returned by [`qjd_parse`] -/// that has not yet been freed. Double-free or passing a pointer not -/// produced by `qjd_parse` is undefined behavior. +/// `doc` must be NULL or a pointer previously returned by [`qjd_parse`] or +/// [`qjd_decoder_parse`] that has not yet been freed. Double-free or passing +/// a pointer not produced by those functions is undefined behavior. #[no_mangle] pub unsafe extern "C" fn qjd_free(doc: *mut qjd_doc) { if doc.is_null() { return; } - let _ = Box::from_raw(doc); + let d = Box::from_raw(doc); + if d.owns_decoder { + let _ = Box::from_raw(d.decoder.as_ptr()); + } +} + +// ── qjd_decoder_* (pooled API) ────────────────────────────────────────────── + +/// Allocate a reusable decoder. Returns NULL on allocation failure. +/// +/// # Safety +/// +/// Has no preconditions. The returned pointer must be freed exactly once +/// with [`qjd_decoder_free`]. +#[no_mangle] +pub unsafe extern "C" fn qjd_decoder_new() -> *mut qjd_decoder { + let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + Box::into_raw(Box::new(qjd_decoder(Decoder::new()))) + })); + r.unwrap_or(std::ptr::null_mut()) +} + +/// Free a decoder returned by [`qjd_decoder_new`]. NULL is a no-op. +/// +/// # Safety +/// +/// `dec` must be NULL or a pointer previously returned by +/// [`qjd_decoder_new`] that has not yet been freed. All docs and cursors +/// produced by this decoder must have been freed first, or the caller must +/// ensure they are not used after this call. +#[no_mangle] +pub unsafe extern "C" fn qjd_decoder_free(dec: *mut qjd_decoder) { + if dec.is_null() { return; } + let _ = Box::from_raw(dec); +} + +/// Reset a decoder: drop all cached state and release allocated capacity. +/// The decoder remains usable and its generation advances so any +/// outstanding docs/cursors become stale. +/// +/// # Safety +/// +/// `dec` must be NULL or a pointer previously returned by +/// [`qjd_decoder_new`] that has not yet been freed. +#[no_mangle] +pub unsafe extern "C" fn qjd_decoder_reset(dec: *mut qjd_decoder) { + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + if !dec.is_null() { (*dec).0.reset(); } + })); +} + +/// Permanently retire a decoder. Frees the bulk internal buffers; the +/// decoder struct itself is freed when [`qjd_decoder_free`] is called. +/// After destroy, all subsequent decoder operations return errors and all +/// doc/cursor operations against docs produced by this decoder return +/// `QJD_INVALID_ARG`. +/// +/// # Safety +/// +/// `dec` must be NULL or a pointer previously returned by +/// [`qjd_decoder_new`] that has not yet been freed. +#[no_mangle] +pub unsafe extern "C" fn qjd_decoder_destroy(dec: *mut qjd_decoder) { + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + if !dec.is_null() { (*dec).0.destroy(); } + })); } +/// Parse `buf` into `dec` and return a new doc handle. Any prior doc/cursor +/// produced by this decoder is invalidated (their generation no longer +/// matches and operations on them return `QJD_STALE_DOC`). +/// +/// # Safety +/// +/// - `dec` must be a live decoder pointer returned by [`qjd_decoder_new`]. +/// NULL or a destroyed decoder yields `QJD_INVALID_ARG`. +/// - `buf` must point to `len` readable bytes, or be NULL when `len == 0`. +/// - `err_out` must point to a writable `int`; NULL yields NULL with no +/// error code written. +/// - The buffer must remain valid and unmodified until the next +/// `qjd_decoder_parse` / `_reset` / `_destroy` / `_free` call on `dec`, +/// or any operation on a doc/cursor produced by this parse. +/// - On success, the returned pointer must be freed with [`qjd_free`]. +#[no_mangle] +pub unsafe extern "C" fn qjd_decoder_parse( + dec: *mut qjd_decoder, + buf: *const u8, + len: usize, + err_out: *mut c_int, +) -> *mut qjd_doc { + let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + if err_out.is_null() { return ptr::null_mut(); } + if dec.is_null() || (buf.is_null() && len != 0) { + *err_out = qjd_err::QJD_INVALID_ARG as c_int; + return ptr::null_mut(); + } + if matches!((*dec).0.state, DecoderState::Destroyed) { + *err_out = qjd_err::QJD_INVALID_ARG as c_int; + return ptr::null_mut(); + } + let slice: &[u8] = if buf.is_null() { &[] } else { + std::slice::from_raw_parts(buf, len) + }; + match (*dec).0.parse(slice) { + Ok(()) => { + *err_out = qjd_err::QJD_OK as c_int; + let doc = qjd_doc { + decoder: NonNull::new_unchecked(dec), + gen: (*dec).0.gen, + owns_decoder: false, + }; + Box::into_raw(Box::new(doc)) + } + Err(e) => { + *err_out = e as c_int; + ptr::null_mut() + } + } + })); + match r { + Ok(p) => p, + Err(_) => { + if !err_out.is_null() { *err_out = qjd_err::QJD_OOM as c_int; } + std::ptr::null_mut() + } + } +} + +// ── Root-path resolution helper ───────────────────────────────────────────── + use crate::cursor::Cursor; use crate::error::qjd_type; unsafe fn resolve_root_path( doc: *mut qjd_doc, path: *const c_char, path_len: usize, -) -> Result<(&'static Document<'static>, Cursor), qjd_err> { - if doc.is_null() || (path.is_null() && path_len != 0) { +) -> Result<(&'static Decoder, Cursor), qjd_err> { + if path.is_null() && path_len != 0 { return Err(qjd_err::QJD_INVALID_ARG); } - let d: &Document = &(*doc).0; + let d = check_doc_alive(doc)?; let p: &[u8] = if path.is_null() { &[] } else { std::slice::from_raw_parts(path as *const u8, path_len) }; let cur = Cursor::root(d).resolve(d, p)?; - Ok((std::mem::transmute::<&Document<'_>, &'static Document<'static>>(d), cur)) + Ok((std::mem::transmute::<&Decoder, &'static Decoder>(d), cur)) } +// ── Path-based getters ────────────────────────────────────────────────────── + /// Write the JSON value type at `path` into `*type_out` (see [`qjd_type`]). /// /// # Safety @@ -344,7 +537,7 @@ pub unsafe extern "C" fn qjd_get_bool( /// Return the byte slice for a scalar value (number, true, false, null). /// Uses the cursor convention: cur.idx_start is the position in indices of /// the structural char AFTER the scalar (a separator or closer). -unsafe fn scalar_bytes<'d>(d: &'d Document<'d>, cur: Cursor) -> Result<&'d [u8], qjd_err> { +unsafe fn scalar_bytes(d: &Decoder, cur: Cursor) -> Result<&[u8], qjd_err> { // First byte: just after the previous structural char (skip whitespace). let start = d.find_scalar_start(cur.idx_start)?; // End byte: position of the structural char at cur.idx_start (exclusive). @@ -368,16 +561,14 @@ pub struct qjd_cursor { pub _reserved1: u32, } -/// Turn a `*const qjd_cursor` into `(&'static Document<'static>, Cursor)` for Rust use. -unsafe fn cursor_to_internal(c: *const qjd_cursor) -> Result<(&'static Document<'static>, Cursor), qjd_err> { +/// Turn a `*const qjd_cursor` into `(&Decoder, Cursor)` for Rust use, after +/// validating both the doc handle and the gen against the underlying decoder. +unsafe fn cursor_to_internal(c: *const qjd_cursor) -> Result<(&'static Decoder, Cursor), qjd_err> { if c.is_null() { return Err(qjd_err::QJD_INVALID_ARG); } let cc = &*c; if cc.doc.is_null() { return Err(qjd_err::QJD_INVALID_ARG); } - let d: &Document = &(*(cc.doc as *mut qjd_doc)).0; - Ok(( - std::mem::transmute::<&Document<'_>, &'static Document<'static>>(d), - Cursor { idx_start: cc.idx_start, idx_end: cc.idx_end }, - )) + let d = check_doc_alive(cc.doc as *mut qjd_doc)?; + Ok((d, Cursor { idx_start: cc.idx_start, idx_end: cc.idx_end })) } fn internal_to_cursor(doc: *const qjd_doc, cur: Cursor) -> qjd_cursor { diff --git a/src/lib.rs b/src/lib.rs index 43a07ca..3db292d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,7 +4,7 @@ pub mod error; pub(crate) mod scan; mod skip_cache; -mod doc; +mod decoder; mod path; mod cursor; mod decode; diff --git a/src/skip_cache.rs b/src/skip_cache.rs index 6b6b1dd..354843b 100644 --- a/src/skip_cache.rs +++ b/src/skip_cache.rs @@ -49,4 +49,18 @@ impl SkipCache { #[cfg(test)] pub(crate) fn len(&self) -> usize { self.by_opener.len() } + + /// Drop every cached entry but keep allocated capacity. + pub(crate) fn clear(&mut self) { + self.slots.truncate(1); + self.slots[0].child_starts.clear(); + self.slots[0].child_ends.clear(); + self.by_opener.clear(); + } + + /// Drop every cached entry and release allocated capacity back to the allocator. + pub(crate) fn clear_and_shrink(&mut self) { + // Replacing entirely is the cheapest way to drop both Vecs and the FxHashMap. + *self = SkipCache::new(); + } } From ff28fb45a1ae669394c62ad942dac23c60eee29e Mon Sep 17 00:00:00 2001 From: Yuansheng Wang Date: Fri, 15 May 2026 17:43:22 +0000 Subject: [PATCH 3/4] test+wrapper: decoder Lua surface, FFI/alloc tests, busted spec (#6) - lua/quickdecode.lua: new_decoder + Decoder:parse/reset/destroy with the STALE_DOC -> nil convention. Doc table pins _decoder so the decoder outlives any reachable doc, and the decoder pins _payload so the current input buffer outlives the parse. - tests/decoder_ffi.rs (14 tests): equivalence between qjd_parse and qjd_decoder_parse on shipped fixtures; stale-doc / stale-cursor / reset / destroy semantics; legacy path isolation. - tests/alloc_count.rs (count-allocs feature): head-to-head allocation count between legacy and pooled. Asserts pooled < legacy / 2. On the medium fixture: legacy ~10/parse, pooled 2/parse. - tests/lua/decoder_spec.lua: busted spec covering parse/reset/destroy, stale-as-nil, cursor staleness, legacy-isolation. - .github/workflows/ci.yml: add count-allocs matrix point. - README: pooled-API usage section + two new Roadmap / Deferred items. - Cargo.toml: count-allocs feature. Spec updated to drop the now-unneeded qjd_cursor gen field (cursor freshness derives from its doc.gen via check_doc_alive). --- .github/workflows/ci.yml | 3 + Cargo.toml | 7 +- README.md | 23 ++ .../2026-05-15-decoder-pooling-design.md | 20 +- lua/quickdecode.lua | 53 ++- src/decoder.rs | 6 +- tests/alloc_count.rs | 125 +++++++ tests/decoder_ffi.rs | 335 ++++++++++++++++++ tests/lua/decoder_spec.lua | 111 ++++++ 9 files changed, 662 insertions(+), 21 deletions(-) create mode 100644 tests/alloc_count.rs create mode 100644 tests/decoder_ffi.rs create mode 100644 tests/lua/decoder_spec.lua diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd39c2a..06eeb4e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,9 @@ jobs: - name: Test with test-panic feature run: cargo test --features test-panic --release + - name: Test allocation accounting (count-allocs feature) + run: cargo test --release --features count-allocs --test alloc_count + lua: name: Lua integration tests runs-on: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index 98433a9..ad518da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,9 +9,10 @@ name = "quickdecode" crate-type = ["cdylib", "rlib"] [features] -default = ["avx2"] -avx2 = [] -test-panic = [] +default = ["avx2"] +avx2 = [] +test-panic = [] +count-allocs = [] [dependencies] memchr = "2" diff --git a/README.md b/README.md index b2fdd3d..9f0de27 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,26 @@ local model = body:get_str("model") local temp = body:get_f64("temperature") ``` +### Reusable decoder (pooled API) + +For hot paths that parse many payloads (typical in OpenResty workers), use a +reusable decoder to amortize the per-parse indices / scratch / skip-cache +allocations: + +```lua +local decoder = qd.new_decoder() -- one per worker is enough +for _, payload in ipairs(payloads) do + local doc = decoder:parse(payload) + -- ...access doc / open cursors... +end +decoder:reset() -- optional: shrink internal buffers +decoder:destroy() -- optional: free buffers eagerly +``` + +A `doc` returned by `decoder:parse()` becomes stale as soon as the same +decoder parses another payload (or is reset / destroyed). Accessor calls on a +stale doc return `nil`, the same convention as a missing path. + ## Testing — Lua Requires LuaJIT + busted + lua-cjson installed system-wide. @@ -76,3 +96,6 @@ Items intentionally pushed out of the first implementation. Each will be picked - **`cargo fmt --check` not enforced** — `make lint` runs clippy only. The codebase uses intentional manual column alignment in struct definitions and compact single-line literals that default rustfmt would reflow. Skip rather than reformat until a project-wide style decision is made. - **`validate_brackets` fusion into scan emit loop** — surfaced by profiling: on structurally-dense workloads `validate_brackets` is 65% of parse time (second linear pass over emitted indices). Folding bracket pairing into the scan emit loop via an inline depth stack eliminates that pass. No effect on the current string-heavy bench (0.3% there); a win for config / JSONL / table-shape JSON. - **`memchr2` cross-chunk jump for very long string interiors** — the AVX2 in-string fast probe (issue #5) drops per-chunk cost from ~25 to ~10 ops but still pays ALU work for every 64-byte chunk in a string. A `memchr2(b'"', b'\\')` jump can approach memory bandwidth on multi-MB single-string payloads. Deferred until a workload that benefits clearly emerges; needs careful `bs_carry` reasoning across the jump. +- **Eliminate `validate_brackets` per-scan stack alloc on the pooled path** — the bracket-balance check builds a fresh `Vec::with_capacity(32)` every scan. On the pooled decoder API this and the per-parse `Box` are the only allocations the count-allocs test still sees (2 / parse). A pre-allocated stack on the `Decoder` would drop the count further; deferred because the absolute cost is tiny and the cleanest fix overlaps with the `validate_brackets` fusion item above. +- **Decoder pool / shared-decoder shortcut for `qd.parse`** — `qd.parse(payload)` still constructs a private decoder per call (1 indices Vec + 1 scratch + 1 skip-cache alloc each). A module-level shared decoder could make the legacy API allocation-free too, but adds a global-state footgun (no concurrent parses from coroutines); decoder pooling is exposed via the explicit `qd.new_decoder()` API instead. Reconsider if profiling shows `qd.parse` callers refusing to migrate. +- **Decoder generation counter wrap** — after `2^32` parses on the same decoder the gen wraps to a value an old (Lua-GC-still-alive) doc might match, masking staleness. With 1 ms/parse that is ~50 days of continuous reuse; in practice the doc is reclaimed long before. Could widen to `u64` or trip a hard error near the wrap point if a real-world workload comes close. diff --git a/docs/superpowers/specs/2026-05-15-decoder-pooling-design.md b/docs/superpowers/specs/2026-05-15-decoder-pooling-design.md index cdf4b4c..f71cc7c 100644 --- a/docs/superpowers/specs/2026-05-15-decoder-pooling-design.md +++ b/docs/superpowers/specs/2026-05-15-decoder-pooling-design.md @@ -84,7 +84,7 @@ void qjd_decoder_destroy(qjd_decoder*); qjd_doc* qjd_decoder_parse(qjd_decoder*, const uint8_t* buf, size_t len, int* err_out); ``` -The returned `qjd_doc*` is the same opaque type as today. All existing `qjd_get_*`, `qjd_open`, and `qjd_cursor_*` functions accept it. The cursor struct gains a `gen` field by repurposing one of its `_reserved` slots — see §4. +The returned `qjd_doc*` is the same opaque type as today. All existing `qjd_get_*`, `qjd_open`, and `qjd_cursor_*` functions accept it. The cursor struct is unchanged; its freshness check is derived through its `doc` pointer — see §4.4. A new error code is added to `src/error.rs` and the header: @@ -207,21 +207,17 @@ pub struct qjd_doc { ### 4.4 Cursor -The `qjd_cursor` C struct currently has two `_reserved` fields. One is repurposed: +The `qjd_cursor` C struct is unchanged — both `_reserved0` and `_reserved1` stay reserved. A cursor's freshness is derived through its `doc` pointer: -```c -typedef struct { - const qjd_doc* doc; - uint32_t idx_start; - uint32_t idx_end; - uint32_t gen; // was _reserved0 - uint32_t _reserved1; -} qjd_cursor; +```rust +// In cursor_to_internal: +let doc: &qjd_doc = &*(c.doc as *mut qjd_doc); // already pinned by Lua wrapper's _doc ref +check_doc_alive(c.doc as *mut qjd_doc)?; // doc.gen vs decoder.gen + state check ``` -`qjd_open` / `qjd_cursor_open` / `qjd_cursor_field` / `qjd_cursor_index` populate `gen` from the doc's current decoder gen. Every cursor accessor checks `cursor.gen == decoder.gen` before dereferencing. +Since the Lua wrapper's `Cursor` table keeps a strong `_doc = self._doc` reference (preserving today's pattern), `cursor.doc` is always a valid pointer while the cursor is reachable. The gen check on the doc handles staleness for both the doc itself and any cursor opened from it: once the decoder reparses, both the doc and all its cursors fail the gen check. -The Lua wrapper traffics `qjd_cursor` by value through `cur_box`; no Lua-side changes are required. +No ABI change to `qjd_cursor`. ### 4.5 Refactor mechanics diff --git a/lua/quickdecode.lua b/lua/quickdecode.lua index 9675231..f512eda 100644 --- a/lua/quickdecode.lua +++ b/lua/quickdecode.lua @@ -2,6 +2,7 @@ local ffi = require("ffi") ffi.cdef[[ typedef struct qjd_doc qjd_doc; +typedef struct qjd_decoder qjd_decoder; typedef struct { const qjd_doc* doc; uint32_t idx_start, idx_end, _reserved0, _reserved1; @@ -11,6 +12,12 @@ const char* qjd_strerror(int code); qjd_doc* qjd_parse(const uint8_t* buf, size_t len, int* err_out); void qjd_free(qjd_doc* doc); +qjd_decoder* qjd_decoder_new(void); +void qjd_decoder_free(qjd_decoder*); +void qjd_decoder_reset(qjd_decoder*); +void qjd_decoder_destroy(qjd_decoder*); +qjd_doc* qjd_decoder_parse(qjd_decoder*, const uint8_t*, size_t, int*); + int qjd_get_str (qjd_doc*, const char* path, size_t path_len, const uint8_t** p, size_t* n); int qjd_get_i64 (qjd_doc*, const char* path, size_t path_len, int64_t* out); int qjd_get_f64 (qjd_doc*, const char* path, size_t path_len, double* out); @@ -44,18 +51,24 @@ local strp_box = ffi.new("const uint8_t*[1]") local cur_box = ffi.new("qjd_cursor[1]") local NOT_FOUND = 2 +local STALE_DOC = 9 local _M = { T_NULL = 0, T_BOOL = 1, T_NUM = 2, T_STR = 3, T_ARR = 4, T_OBJ = 5, } -local Doc = {}; Doc.__index = Doc -local Cursor = {}; Cursor.__index = Cursor +local Doc = {}; Doc.__index = Doc +local Cursor = {}; Cursor.__index = Cursor +local Decoder = {}; Decoder.__index = Decoder +-- check_err returns: +-- true for QJD_OK +-- false for QJD_NOT_FOUND / QJD_STALE_DOC (callers translate to nil) +-- raises for every other code local function check_err(rc) if rc == 0 then return true end - if rc == NOT_FOUND then return false end + if rc == NOT_FOUND or rc == STALE_DOC then return false end error("quickdecode: " .. ffi.string(C.qjd_strerror(rc))) end @@ -70,6 +83,40 @@ function _M.parse(json_str) }, Doc) end +function _M.new_decoder() + local ptr = C.qjd_decoder_new() + if ptr == nil then + error("quickdecode: decoder allocation failed") + end + return setmetatable({ + _ptr = ffi.gc(ptr, C.qjd_decoder_free), + }, Decoder) +end + +function Decoder:parse(payload) + -- Pin the current payload against Lua GC while it's borrowed by the + -- decoder. Replacing _payload on each parse drops the previous payload, + -- which is safe because the previous doc is now stale (gen bumped) and + -- cannot be dereferenced. + self._payload = payload + local doc_ptr = C.qjd_decoder_parse(self._ptr, payload, #payload, err_box) + if doc_ptr == nil then + error("quickdecode: " .. ffi.string(C.qjd_strerror(err_box[0]))) + end + return setmetatable({ + _ptr = ffi.gc(doc_ptr, C.qjd_free), + _decoder = self, -- keep decoder (and thus _payload) alive + }, Doc) +end + +function Decoder:reset() + C.qjd_decoder_reset(self._ptr) +end + +function Decoder:destroy() + C.qjd_decoder_destroy(self._ptr) +end + function Doc:get_str(path) local rc = C.qjd_get_str(self._ptr, path, #path, strp_box, size_box) if not check_err(rc) then return nil end diff --git a/src/decoder.rs b/src/decoder.rs index 1da29bb..318b0ea 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -103,9 +103,9 @@ impl Decoder { self.state = DecoderState::Destroyed; } - /// Convenience for tests and the legacy `qjd_parse` path: construct a - /// decoder and parse `input` in one call. - pub fn parse_oneshot(input: &[u8]) -> Result { + /// Convenience for tests: construct a decoder and parse `input` in one call. + #[cfg(test)] + pub(crate) fn parse_oneshot(input: &[u8]) -> Result { let mut d = Self::new(); d.parse(input)?; Ok(d) diff --git a/tests/alloc_count.rs b/tests/alloc_count.rs new file mode 100644 index 0000000..e46c952 --- /dev/null +++ b/tests/alloc_count.rs @@ -0,0 +1,125 @@ +//! Allocation-count regression test for the pooled decoder API. +//! +//! Installs a counting `GlobalAlloc` and runs both APIs across N parses, +//! asserting: +//! 1. The legacy `qjd_parse` path allocates at least once per call. +//! 2. The pooled `qjd_decoder_parse` path allocates strictly less than +//! the legacy path — the indices / scratch / skip-cache Vecs and the +//! decoder Box are no longer rebuilt per parse. +//! +//! There are unavoidable per-parse allocations the pooled path still +//! incurs (the `Box` handle, the bracket-balance check's +//! `Vec::with_capacity(32)` stack in `validate_brackets`). They are small +//! and fixed-size, so the assertion uses a ratio rather than a hard +//! ceiling: pooled must be at most half of legacy. +//! +//! Gated behind the `count-allocs` Cargo feature because swapping the +//! global allocator is process-wide and would interfere with other tests. + +#![cfg(feature = "count-allocs")] + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::os::raw::c_int; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use quickdecode::ffi::*; + +struct CountingAlloc { + inner: System, + count: AtomicUsize, +} + +impl CountingAlloc { + const fn new() -> Self { + Self { inner: System, count: AtomicUsize::new(0) } + } + fn count(&self) -> usize { self.count.load(Ordering::Relaxed) } +} + +unsafe impl GlobalAlloc for CountingAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + self.count.fetch_add(1, Ordering::Relaxed); + self.inner.alloc(layout) + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + self.inner.dealloc(ptr, layout) + } +} + +#[global_allocator] +static ALLOC: CountingAlloc = CountingAlloc::new(); + +const PAYLOAD: &[u8] = include_bytes!("../benches/fixtures/medium_resp.json"); +const ITERS: usize = 1_000; + +fn count_legacy() -> usize { + unsafe { + // Warmup allocator caches. + for _ in 0..5 { + let mut err: c_int = -1; + let doc = qjd_parse(PAYLOAD.as_ptr(), PAYLOAD.len(), &mut err); + qjd_free(doc); + } + let baseline = ALLOC.count(); + for _ in 0..ITERS { + let mut err: c_int = -1; + let doc = qjd_parse(PAYLOAD.as_ptr(), PAYLOAD.len(), &mut err); + assert!(!doc.is_null()); + qjd_free(doc); + } + ALLOC.count() - baseline + } +} + +fn count_pooled() -> usize { + unsafe { + let dec = qjd_decoder_new(); + assert!(!dec.is_null()); + // Warmup: let Vec capacities reach steady state. + for _ in 0..5 { + let mut err: c_int = -1; + let doc = qjd_decoder_parse(dec, PAYLOAD.as_ptr(), PAYLOAD.len(), &mut err); + assert!(!doc.is_null()); + qjd_free(doc); + } + let baseline = ALLOC.count(); + for _ in 0..ITERS { + let mut err: c_int = -1; + let doc = qjd_decoder_parse(dec, PAYLOAD.as_ptr(), PAYLOAD.len(), &mut err); + assert!(!doc.is_null()); + qjd_free(doc); + } + let delta = ALLOC.count() - baseline; + qjd_decoder_free(dec); + delta + } +} + +#[test] +fn legacy_path_allocates_per_parse() { + let delta = count_legacy(); + assert!( + delta >= ITERS, + "legacy path allocated only {} times across {} iterations (expected >= {})", + delta, ITERS, ITERS + ); +} + +#[test] +fn pooled_path_uses_fewer_allocations_than_legacy() { + let legacy = count_legacy(); + let pooled = count_pooled(); + + // Pooled should be at most half of legacy. Looser than "near zero" but + // robust to small per-iter allocations we cannot avoid (Box, + // validate_brackets stack) without a bigger refactor. + let ceiling = legacy / 2; + assert!( + pooled < ceiling, + "pooled={} legacy={} ceiling={} (pooled must be < legacy/2)", + pooled, legacy, ceiling + ); + + // Emit the absolute numbers so CI logs make regressions visible. + eprintln!("alloc_count: legacy={} pooled={} (across {} iters)", legacy, pooled, ITERS); +} diff --git a/tests/decoder_ffi.rs b/tests/decoder_ffi.rs new file mode 100644 index 0000000..653bf46 --- /dev/null +++ b/tests/decoder_ffi.rs @@ -0,0 +1,335 @@ +//! FFI-level integration tests for the pooled decoder API. +//! +//! Covers: +//! - Equivalence between the legacy `qjd_parse` path and the new +//! `qjd_decoder_parse` path against the shipped fixtures. +//! - Stale-doc detection across re-parses, resets, and destroys. +//! - Cursor staleness derived through the doc's gen. +//! - Destroyed-decoder rejection. + +use std::os::raw::c_int; +use std::ptr; + +use quickdecode::ffi::*; + +const FIXTURE_SMALL: &[u8] = include_bytes!("../benches/fixtures/small_api.json"); +const FIXTURE_MEDIUM: &[u8] = include_bytes!("../benches/fixtures/medium_resp.json"); + +// ── helpers ──────────────────────────────────────────────────────────────── + +fn dec_new() -> *mut qjd_decoder { + let p = unsafe { qjd_decoder_new() }; + assert!(!p.is_null()); + p +} + +fn dec_parse(dec: *mut qjd_decoder, buf: &[u8]) -> *mut qjd_doc { + let mut err: c_int = -1; + let d = unsafe { qjd_decoder_parse(dec, buf.as_ptr(), buf.len(), &mut err) }; + assert_eq!(err, 0, "qjd_decoder_parse unexpected err {}", err); + assert!(!d.is_null()); + d +} + +fn legacy_parse(buf: &[u8]) -> *mut qjd_doc { + let mut err: c_int = -1; + let d = unsafe { qjd_parse(buf.as_ptr(), buf.len(), &mut err) }; + assert_eq!(err, 0); + assert!(!d.is_null()); + d +} + +/// Try get_str. Returns (rc, value-on-success). +unsafe fn try_get_str(doc: *mut qjd_doc, path: &str) -> (c_int, Option>) { + let mut p: *const u8 = ptr::null(); + let mut n: usize = 0; + let rc = qjd_get_str(doc, path.as_ptr() as *const i8, path.len(), &mut p, &mut n); + let v = if rc == 0 { Some(std::slice::from_raw_parts(p, n).to_vec()) } else { None }; + (rc, v) +} + +unsafe fn try_get_f64(doc: *mut qjd_doc, path: &str) -> (c_int, f64) { + let mut v: f64 = 0.0; + let rc = qjd_get_f64(doc, path.as_ptr() as *const i8, path.len(), &mut v); + (rc, v) +} + +unsafe fn try_typeof(doc: *mut qjd_doc, path: &str) -> (c_int, c_int) { + let mut t: c_int = -1; + let rc = qjd_typeof(doc, path.as_ptr() as *const i8, path.len(), &mut t); + (rc, t) +} + +unsafe fn try_len(doc: *mut qjd_doc, path: &str) -> (c_int, usize) { + let mut n: usize = 0; + let rc = qjd_len(doc, path.as_ptr() as *const i8, path.len(), &mut n); + (rc, n) +} + +/// Sweep a fixture's interesting paths and assert two docs agree on every +/// accessor return — both the rc and the produced value where rc == 0. +unsafe fn assert_doc_equivalence(legacy: *mut qjd_doc, pooled: *mut qjd_doc, probes: &[&str]) { + for p in probes { + let (l_rc, l_v) = try_get_str(legacy, p); + let (r_rc, r_v) = try_get_str(pooled, p); + assert_eq!((l_rc, &l_v), (r_rc, &r_v), "get_str mismatch at {}", p); + + let (l_rc, l_v) = try_get_f64(legacy, p); + let (r_rc, r_v) = try_get_f64(pooled, p); + assert_eq!(l_rc, r_rc, "get_f64 rc mismatch at {}", p); + if l_rc == 0 { + assert_eq!(l_v.to_bits(), r_v.to_bits(), "get_f64 value mismatch at {}", p); + } + + let (l_rc, l_t) = try_typeof(legacy, p); + let (r_rc, r_t) = try_typeof(pooled, p); + assert_eq!((l_rc, l_t), (r_rc, r_t), "typeof mismatch at {}", p); + + let (l_rc, l_n) = try_len(legacy, p); + let (r_rc, r_n) = try_len(pooled, p); + assert_eq!((l_rc, l_n), (r_rc, r_n), "len mismatch at {}", p); + } +} + +// ── equivalence with the legacy qjd_parse path ───────────────────────────── + +#[test] +fn decoder_path_matches_legacy_on_small_fixture() { + let probes = &[ + "model", "temperature", "max_tokens", "top_p", "stream", + "messages", "messages[0].role", "messages[0].content", + "missing", "messages[100]", + ]; + unsafe { + let legacy = legacy_parse(FIXTURE_SMALL); + let dec = dec_new(); + let pooled = dec_parse(dec, FIXTURE_SMALL); + assert_doc_equivalence(legacy, pooled, probes); + qjd_free(legacy); + qjd_free(pooled); + qjd_decoder_free(dec); + } +} + +#[test] +fn decoder_path_matches_legacy_on_medium_fixture() { + let probes = &[ + "id", "object", "created", "model", + "choices", "choices[0].index", "choices[0].message.role", + "choices[0].message.content", "choices[0].finish_reason", + "usage", "usage.prompt_tokens", "usage.completion_tokens", + "missing.path", "choices[99]", + ]; + unsafe { + let legacy = legacy_parse(FIXTURE_MEDIUM); + let dec = dec_new(); + let pooled = dec_parse(dec, FIXTURE_MEDIUM); + assert_doc_equivalence(legacy, pooled, probes); + qjd_free(legacy); + qjd_free(pooled); + qjd_decoder_free(dec); + } +} + +// ── stale-doc & cursor detection ─────────────────────────────────────────── + +#[test] +fn second_parse_marks_first_doc_stale() { + unsafe { + let dec = dec_new(); + let doc1 = dec_parse(dec, b"{\"a\":1}"); + let doc2 = dec_parse(dec, b"{\"b\":2}"); + + // doc2 is current and works. + let mut v: i64 = 0; + let rc = qjd_get_i64(doc2, b"b".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 0); + assert_eq!(v, 2); + + // doc1 is stale. + let rc = qjd_get_i64(doc1, b"a".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 9, "expected QJD_STALE_DOC, got {}", rc); + + qjd_free(doc1); + qjd_free(doc2); + qjd_decoder_free(dec); + } +} + +#[test] +fn cursor_opened_before_reparse_becomes_stale() { + unsafe { + let dec = dec_new(); + let doc1 = dec_parse(dec, b"{\"a\":1,\"b\":[10,20,30]}"); + + let mut cur = std::mem::MaybeUninit::::uninit(); + let rc = qjd_open(doc1, b"b".as_ptr() as *const i8, 1, cur.as_mut_ptr()); + assert_eq!(rc, 0); + let cur = cur.assume_init(); + + // Reparse. Cursor opened against doc1 must now report stale. + let doc2 = dec_parse(dec, b"{\"a\":2}"); + let mut n: usize = 0; + let rc = qjd_cursor_len(&cur, ptr::null(), 0, &mut n); + assert_eq!(rc, 9, "expected QJD_STALE_DOC, got {}", rc); + + qjd_free(doc1); + qjd_free(doc2); + qjd_decoder_free(dec); + } +} + +#[test] +fn reset_invalidates_outstanding_doc() { + unsafe { + let dec = dec_new(); + let doc = dec_parse(dec, b"{\"a\":1}"); + qjd_decoder_reset(dec); + + let mut v: i64 = 0; + let rc = qjd_get_i64(doc, b"a".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 9, "expected QJD_STALE_DOC after reset, got {}", rc); + + qjd_free(doc); + qjd_decoder_free(dec); + } +} + +#[test] +fn decoder_is_reusable_after_reset() { + unsafe { + let dec = dec_new(); + let doc1 = dec_parse(dec, b"{\"a\":1}"); + qjd_free(doc1); + qjd_decoder_reset(dec); + + // Re-use is fine and the new doc works. + let doc2 = dec_parse(dec, b"{\"b\":42}"); + let mut v: i64 = 0; + let rc = qjd_get_i64(doc2, b"b".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 0); + assert_eq!(v, 42); + + qjd_free(doc2); + qjd_decoder_free(dec); + } +} + +// ── destroy semantics ────────────────────────────────────────────────────── + +#[test] +fn destroyed_decoder_rejects_parse() { + unsafe { + let dec = dec_new(); + qjd_decoder_destroy(dec); + + let mut err: c_int = -1; + let d = qjd_decoder_parse(dec, b"{}".as_ptr(), 2, &mut err); + assert!(d.is_null()); + assert_eq!(err, 7, "expected QJD_INVALID_ARG, got {}", err); + + qjd_decoder_free(dec); + } +} + +#[test] +fn destroyed_decoder_rejects_doc_ops_with_invalid_arg() { + // Doc operations after destroy must return QJD_INVALID_ARG (terminal + // state takes precedence over the gen check). + unsafe { + let dec = dec_new(); + let doc = dec_parse(dec, b"{\"a\":1}"); + qjd_decoder_destroy(dec); + + let mut v: i64 = 0; + let rc = qjd_get_i64(doc, b"a".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 7, "expected QJD_INVALID_ARG after destroy, got {}", rc); + + qjd_free(doc); + qjd_decoder_free(dec); + } +} + +#[test] +fn destroy_is_idempotent_across_ffi() { + unsafe { + let dec = dec_new(); + qjd_decoder_destroy(dec); + qjd_decoder_destroy(dec); // second call is a no-op + qjd_decoder_free(dec); + } +} + +// ── error paths ──────────────────────────────────────────────────────────── + +#[test] +fn null_decoder_yields_invalid_arg() { + let mut err: c_int = -1; + let d = unsafe { qjd_decoder_parse(ptr::null_mut(), b"{}".as_ptr(), 2, &mut err) }; + assert!(d.is_null()); + assert_eq!(err, 7); +} + +#[test] +fn null_err_out_yields_null_silent() { + let dec = dec_new(); + let d = unsafe { + qjd_decoder_parse(dec, b"{}".as_ptr(), 2, ptr::null_mut()) + }; + assert!(d.is_null()); + unsafe { qjd_decoder_free(dec); } +} + +#[test] +fn parse_error_does_not_destroy_decoder() { + unsafe { + let dec = dec_new(); + let mut err: c_int = -1; + let d = qjd_decoder_parse(dec, b"{".as_ptr(), 1, &mut err); + assert!(d.is_null()); + assert_eq!(err, 1, "expected QJD_PARSE_ERROR, got {}", err); + + // Decoder remains usable; a subsequent valid parse succeeds. + let d = qjd_decoder_parse(dec, b"{\"x\":7}".as_ptr(), 7, &mut err); + assert!(!d.is_null()); + assert_eq!(err, 0); + qjd_free(d); + qjd_decoder_free(dec); + } +} + +#[test] +fn free_null_doc_and_decoder_is_safe() { + unsafe { + qjd_free(ptr::null_mut()); + qjd_decoder_free(ptr::null_mut()); + qjd_decoder_reset(ptr::null_mut()); + qjd_decoder_destroy(ptr::null_mut()); + } +} + +#[test] +fn legacy_parse_doc_works_independently_of_decoder_api() { + // Sanity: even after spinning up a separate decoder, the legacy path + // is unaffected. + unsafe { + let dec = dec_new(); + let doc1 = dec_parse(dec, b"{\"x\":1}"); + let doc2 = legacy_parse(b"{\"y\":2}"); + + let mut v: i64 = 0; + let rc = qjd_get_i64(doc2, b"y".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 0); + assert_eq!(v, 2); + + // Re-parsing on the decoder must not stale-out the legacy doc. + let _doc3 = dec_parse(dec, b"{\"z\":3}"); + let rc = qjd_get_i64(doc2, b"y".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 0, "legacy doc must remain valid; got rc {}", rc); + + qjd_free(doc1); + qjd_free(doc2); + qjd_free(_doc3); + qjd_decoder_free(dec); + } +} diff --git a/tests/lua/decoder_spec.lua b/tests/lua/decoder_spec.lua new file mode 100644 index 0000000..50a33ea --- /dev/null +++ b/tests/lua/decoder_spec.lua @@ -0,0 +1,111 @@ +local qd = require("quickdecode") + +describe("quickdecode decoder pooling", function() + it("new_decoder returns a usable object", function() + local d = qd.new_decoder() + assert.is_not_nil(d) + assert.are.equal("function", type(d.parse)) + assert.are.equal("function", type(d.reset)) + assert.are.equal("function", type(d.destroy)) + end) + + it("parse returns a Doc supporting the full accessor surface", function() + local dec = qd.new_decoder() + local doc = dec:parse('{"name":"alice","age":30,"active":true}') + + assert.are.equal("alice", doc:get_str("name")) + assert.are.equal(30, doc:get_i64("age")) + assert.is_true(doc:get_bool("active")) + assert.are.equal(qd.T_OBJ, doc:typeof("")) + assert.are.equal(3, doc:len("")) + end) + + it("reuses the decoder across multiple parses", function() + local dec = qd.new_decoder() + for i = 1, 5 do + local doc = dec:parse(string.format('{"i":%d}', i)) + assert.are.equal(i, doc:get_i64("i")) + end + end) + + it("second parse marks the first doc stale (returns nil)", function() + local dec = qd.new_decoder() + local doc1 = dec:parse('{"a":1}') + assert.are.equal(1, doc1:get_i64("a")) + + local doc2 = dec:parse('{"b":2}') + assert.are.equal(2, doc2:get_i64("b")) + + -- doc1 is stale; FFI returns QJD_STALE_DOC which the wrapper turns + -- into nil (same convention as path-not-found). + assert.is_nil(doc1:get_i64("a")) + assert.is_nil(doc1:get_str("a")) + end) + + it("reset invalidates outstanding docs and keeps the decoder usable", function() + local dec = qd.new_decoder() + local doc = dec:parse('{"x":42}') + assert.are.equal(42, doc:get_i64("x")) + + dec:reset() + assert.is_nil(doc:get_i64("x")) + + local doc2 = dec:parse('{"y":7}') + assert.are.equal(7, doc2:get_i64("y")) + end) + + it("destroy makes the decoder reject further parses", function() + local dec = qd.new_decoder() + dec:destroy() + assert.has_error(function() dec:parse('{}') end) + end) + + it("destroy invalidates outstanding docs (raises rather than nil)", function() + -- Per design: post-destroy doc operations get QJD_INVALID_ARG, not + -- QJD_STALE_DOC. The wrapper raises on QJD_INVALID_ARG. + local dec = qd.new_decoder() + local doc = dec:parse('{"a":1}') + dec:destroy() + assert.has_error(function() doc:get_i64("a") end) + end) + + it("legacy qd.parse is not affected by decoder activity", function() + local dec = qd.new_decoder() + local pooled_doc = dec:parse('{"x":1}') + local oneshot = qd.parse('{"y":2}') + + -- Reparse the decoder; the one-shot doc must keep working. + dec:parse('{"z":3}') + assert.are.equal(2, oneshot:get_i64("y")) + assert.is_nil(pooled_doc:get_i64("x")) -- pooled doc became stale + end) + + it("cursors opened from a doc become stale after reparse", function() + local dec = qd.new_decoder() + local doc = dec:parse('{"arr":[10,20,30]}') + local cur = doc:open("arr") + assert.are.equal(3, cur:len()) + + dec:parse('{"other":true}') + assert.is_nil(cur:len()) + assert.is_nil(cur:get_i64("[0]")) + end) + + it("parse error does not poison the decoder", function() + local dec = qd.new_decoder() + assert.has_error(function() dec:parse('{') end) + + local doc = dec:parse('{"ok":1}') + assert.are.equal(1, doc:get_i64("ok")) + end) + + it("repeated reset and destroy on the same decoder are safe", function() + local dec = qd.new_decoder() + dec:reset() + dec:reset() + dec:destroy() + dec:destroy() + -- After destroy: still safe to call, just raises on parse. + assert.has_error(function() dec:parse('{}') end) + end) +end) From 7d9cf4462b944222e81a57ef33f73f5f9cd23a8f Mon Sep 17 00:00:00 2001 From: Yuansheng Wang Date: Fri, 15 May 2026 22:27:48 +0000 Subject: [PATCH 4/4] fix(#6): address PR #11 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Critical: sync include/lua_quick_decode.h with src/error.rs + lua/quickdecode.lua. Add QJD_STALE_DOC enum value and the five qjd_decoder_* prototypes that the public C consumer needs to see. CLAUDE.md requires these three files to stay in lockstep. - Important: harmonize NULL-buf handling. qjd_decoder_parse now rejects NULL even with len == 0, matching qjd_parse and avoiding any reliance on slice::from_raw_parts's edge cases. - Important: add doc_held_across_failed_parse_is_stale (FFI-level) to prove the actual safety claim — a held doc fails the gen check after a failed parse, not merely that state/gen invariants hold inside the decoder. - Important: extend equivalence coverage with two probes hitting the categories the existing fixture probes did not exercise: escape-heavy strings (scratch reuse) and 5-6 level nested object with repeated key access (skip-cache reuse). - Important: cursor_opened_before_reparse_becomes_stale now also opens a fresh cursor on the post-reparse doc and walks it, proving the stale check does not poison the new path. - Add null_buf_rejected_even_with_zero_len for the harmonized qjd_decoder_parse contract. --- include/lua_quick_decode.h | 26 +++++++- src/ffi.rs | 9 ++- tests/decoder_ffi.rs | 132 +++++++++++++++++++++++++++++++++++-- 3 files changed, 155 insertions(+), 12 deletions(-) diff --git a/include/lua_quick_decode.h b/include/lua_quick_decode.h index 0c0c0a0..c6911f0 100644 --- a/include/lua_quick_decode.h +++ b/include/lua_quick_decode.h @@ -17,7 +17,11 @@ typedef enum { QJD_DECODE_FAILED = 5, QJD_INVALID_PATH = 6, QJD_INVALID_ARG = 7, - QJD_OOM = 8 + QJD_OOM = 8, + /* Returned when a qjd_doc* (or a qjd_cursor whose doc field references one) + * was produced by a decoder that has since been re-parsed, reset, or + * destroyed. The handle is no longer usable; obtain a fresh one. */ + QJD_STALE_DOC = 9 } qjd_err; typedef enum { @@ -25,7 +29,8 @@ typedef enum { QJD_T_STR = 3, QJD_T_ARR = 4, QJD_T_OBJ = 5 } qjd_type; -typedef struct qjd_doc qjd_doc; +typedef struct qjd_doc qjd_doc; +typedef struct qjd_decoder qjd_decoder; typedef struct { const qjd_doc* doc; @@ -37,9 +42,26 @@ typedef struct { const char* qjd_strerror(int code); +/* One-shot parse: allocates a private decoder internally; freed by qjd_free. */ qjd_doc* qjd_parse(const uint8_t* buf, size_t len, int* err_out); void qjd_free (qjd_doc* doc); +/* Pooled / reusable decoder. Amortizes per-parse allocations of the + * structural-offset buffer, the lazy-decode scratch buffer, and the skip + * cache across many parses. Recommended for hot paths. + * + * After qjd_decoder_parse() is called on a decoder, all docs and cursors + * produced by *prior* parses on that decoder become stale; operations on + * them return QJD_STALE_DOC. After qjd_decoder_destroy(), all operations + * return QJD_INVALID_ARG. All docs produced by a decoder must be freed + * with qjd_free() before the decoder is freed with qjd_decoder_free(). */ +qjd_decoder* qjd_decoder_new (void); +void qjd_decoder_free (qjd_decoder*); +void qjd_decoder_reset (qjd_decoder*); +void qjd_decoder_destroy(qjd_decoder*); +qjd_doc* qjd_decoder_parse (qjd_decoder*, const uint8_t* buf, size_t len, + int* err_out); + int qjd_get_str (qjd_doc*, const char* path, size_t path_len, const uint8_t** out_ptr, size_t* out_len); int qjd_get_i64 (qjd_doc*, const char* path, size_t path_len, int64_t* out); diff --git a/src/ffi.rs b/src/ffi.rs index 84d2206..603615e 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -263,7 +263,8 @@ pub unsafe extern "C" fn qjd_decoder_destroy(dec: *mut qjd_decoder) { /// /// - `dec` must be a live decoder pointer returned by [`qjd_decoder_new`]. /// NULL or a destroyed decoder yields `QJD_INVALID_ARG`. -/// - `buf` must point to `len` readable bytes, or be NULL when `len == 0`. +/// - `buf` must point to `len` readable bytes. NULL is rejected with +/// `QJD_INVALID_ARG` even when `len == 0`, matching [`qjd_parse`]. /// - `err_out` must point to a writable `int`; NULL yields NULL with no /// error code written. /// - The buffer must remain valid and unmodified until the next @@ -279,7 +280,7 @@ pub unsafe extern "C" fn qjd_decoder_parse( ) -> *mut qjd_doc { let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { if err_out.is_null() { return ptr::null_mut(); } - if dec.is_null() || (buf.is_null() && len != 0) { + if dec.is_null() || buf.is_null() { *err_out = qjd_err::QJD_INVALID_ARG as c_int; return ptr::null_mut(); } @@ -287,9 +288,7 @@ pub unsafe extern "C" fn qjd_decoder_parse( *err_out = qjd_err::QJD_INVALID_ARG as c_int; return ptr::null_mut(); } - let slice: &[u8] = if buf.is_null() { &[] } else { - std::slice::from_raw_parts(buf, len) - }; + let slice: &[u8] = std::slice::from_raw_parts(buf, len); match (*dec).0.parse(slice) { Ok(()) => { *err_out = qjd_err::QJD_OK as c_int; diff --git a/tests/decoder_ffi.rs b/tests/decoder_ffi.rs index 653bf46..1e96999 100644 --- a/tests/decoder_ffi.rs +++ b/tests/decoder_ffi.rs @@ -111,6 +111,67 @@ fn decoder_path_matches_legacy_on_small_fixture() { } } +#[test] +fn decoder_path_matches_legacy_on_escape_heavy_input() { + // Exercises lazy string decode into scratch — the second piece of state + // the pooled decoder reuses. Includes backslash, quote, unicode escape, + // and a surrogate-pair sequence so the scratch buffer is actually used. + // Raw byte strings must be ASCII, so use a raw text string and convert. + // The JSON itself uses \uXXXX escapes for non-ASCII so byte-level + // equivalence between legacy and pooled decode is what we're checking. + let payload: &[u8] = r#"{ + "plain": "no escapes here", + "escaped": "tab\there\nnewline\rcr\\back\"quote", + "unicode": "é中文", + "emoji": "😀 smiley", + "nested": {"deep": {"deeper": {"k": "valueé"}}}, + "arr_of_strs": ["a\nb", "c\\d", "e\"f"] + }"#.as_bytes(); + let probes = &[ + "plain", "escaped", "unicode", "emoji", + "nested.deep.deeper.k", + "arr_of_strs", "arr_of_strs[0]", "arr_of_strs[1]", "arr_of_strs[2]", + "missing", + ]; + unsafe { + let legacy = legacy_parse(payload); + let dec = dec_new(); + let pooled = dec_parse(dec, payload); + assert_doc_equivalence(legacy, pooled, probes); + qjd_free(legacy); + qjd_free(pooled); + qjd_decoder_free(dec); + } +} + +#[test] +fn decoder_path_matches_legacy_on_deeply_nested_input() { + // Exercises the skip cache via repeated traversal of the same container, + // plus a depth (>5 levels) deeper than anything in the shipped fixtures. + let payload: &[u8] = br#"{ + "a": {"b": {"c": {"d": {"e": {"f": 42}}}}}, + "siblings": {"x": 1, "y": 2, "z": 3, "x_again_after_skip": 100} + }"#; + let probes = &[ + "a.b.c.d.e.f", + "a", "a.b", "a.b.c", "a.b.c.d", "a.b.c.d.e", + // Probe same path twice to hit the cached path in walk_children. + "a.b.c.d.e.f", + "siblings.x", "siblings.y", "siblings.z", "siblings.x_again_after_skip", + // And again — cached. + "siblings.x", + ]; + unsafe { + let legacy = legacy_parse(payload); + let dec = dec_new(); + let pooled = dec_parse(dec, payload); + assert_doc_equivalence(legacy, pooled, probes); + qjd_free(legacy); + qjd_free(pooled); + qjd_decoder_free(dec); + } +} + #[test] fn decoder_path_matches_legacy_on_medium_fixture() { let probes = &[ @@ -165,13 +226,62 @@ fn cursor_opened_before_reparse_becomes_stale() { let mut cur = std::mem::MaybeUninit::::uninit(); let rc = qjd_open(doc1, b"b".as_ptr() as *const i8, 1, cur.as_mut_ptr()); assert_eq!(rc, 0); - let cur = cur.assume_init(); + let stale_cur = cur.assume_init(); + + // Reparse with an array root so we can re-open a cursor on doc2. + let doc2 = dec_parse(dec, b"[100,200,300]"); - // Reparse. Cursor opened against doc1 must now report stale. - let doc2 = dec_parse(dec, b"{\"a\":2}"); + // Old cursor must report stale. let mut n: usize = 0; - let rc = qjd_cursor_len(&cur, ptr::null(), 0, &mut n); - assert_eq!(rc, 9, "expected QJD_STALE_DOC, got {}", rc); + let rc = qjd_cursor_len(&stale_cur, ptr::null(), 0, &mut n); + assert_eq!(rc, 9, "old cursor must be stale, got {}", rc); + + // Fresh cursor on doc2 must work — proves the stale check doesn't + // taint the post-reparse path. + let mut cur = std::mem::MaybeUninit::::uninit(); + let rc = qjd_open(doc2, ptr::null(), 0, cur.as_mut_ptr()); + assert_eq!(rc, 0); + let fresh_cur = cur.assume_init(); + let rc = qjd_cursor_len(&fresh_cur, ptr::null(), 0, &mut n); + assert_eq!(rc, 0); + assert_eq!(n, 3); + + let mut v: i64 = 0; + let rc = qjd_cursor_get_i64(&fresh_cur, b"[1]".as_ptr() as *const i8, 3, &mut v); + assert_eq!(rc, 0); + assert_eq!(v, 200); + + qjd_free(doc1); + qjd_free(doc2); + qjd_decoder_free(dec); + } +} + +#[test] +fn doc_held_across_failed_parse_is_stale() { + // The unit test in src/decoder.rs only verifies state/gen invariants + // after a failed parse. This proves the load-bearing behavior: an + // outstanding doc actually fails the gen check at the FFI boundary. + unsafe { + let dec = dec_new(); + let doc1 = dec_parse(dec, b"{\"a\":1}"); + + // Fail-parse on the same decoder. gen must bump even on failure. + let mut err: c_int = -1; + let bad = qjd_decoder_parse(dec, b"{".as_ptr(), 1, &mut err); + assert!(bad.is_null()); + assert_eq!(err, 1, "expected QJD_PARSE_ERROR, got {}", err); + + // doc1 must now report stale, not silently return stale indices. + let mut v: i64 = 0; + let rc = qjd_get_i64(doc1, b"a".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 9, "expected QJD_STALE_DOC after failed parse, got {}", rc); + + // Decoder is still usable: a follow-up valid parse yields a fresh doc. + let doc2 = dec_parse(dec, b"{\"a\":99}"); + let rc = qjd_get_i64(doc2, b"a".as_ptr() as *const i8, 1, &mut v); + assert_eq!(rc, 0); + assert_eq!(v, 99); qjd_free(doc1); qjd_free(doc2); @@ -270,6 +380,18 @@ fn null_decoder_yields_invalid_arg() { assert_eq!(err, 7); } +#[test] +fn null_buf_rejected_even_with_zero_len() { + // Match legacy qjd_parse behavior. slice::from_raw_parts requires a + // non-null pointer even for zero-length slices. + let dec = dec_new(); + let mut err: c_int = -1; + let d = unsafe { qjd_decoder_parse(dec, ptr::null(), 0, &mut err) }; + assert!(d.is_null()); + assert_eq!(err, 7); + unsafe { qjd_decoder_free(dec); } +} + #[test] fn null_err_out_yields_null_silent() { let dec = dec_new();