Commit Graph
8 Commits
Author SHA1 Message Date
Erica FischerandCursor 05b1762077 Fix bugs flagged in code review of jsonpull C++ port
- jsontool.cpp `out()`: route JSON_NUMBER (and anything else non-string)
  through `json_stringify` instead of `o->string()`, which now asserts
  on a non-string type and would crash `--extract` on numeric attributes.
- geojson.{hpp,cpp} `json_end_map`: take `json_pull_ptr` by reference so
  the caller's shared_ptr is released, null-guard before touching
  `jp->source`, and clear `jp->source` after delete to avoid a dangling
  pointer.
- jsonpull/jsonpull.cpp: low-surrogate range check was comparing the
  outer-loop byte `c` instead of the parsed code unit `ch`, breaking
  surrogate-pair decoding for some \\uXXXX escapes. Pre-existing bug
  preserved across the port.
- tile-join.cpp `handle_vector_layers`: require the field value to have
  type JSON_STRING (and the key to be non-null) before calling
  `string()`; the previous truthy `type` check would assert on a
  non-string value.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-30 17:49:28 -07:00
Erica FischerandCursor 1bf18d39ce Discriminate json_number's three numeric slots into one union
json_number used to carry three parallel 8-byte fields (a double plus
both a 64-bit unsigned and a 64-bit signed slot for the large-integer
cases) even though at most one of the integer slots is ever the
canonical value for any given number. Collapse them into a
discriminated union:

    enum repr_t { REPR_DOUBLE, REPR_LARGE_UNSIGNED, REPR_LARGE_SIGNED };
    repr_t repr;
    union { double d; unsigned long long u; long long s; } value;

Callers keep the same read API: number() returns the appropriate
double, large_unsigned() returns the ull (or 0 if not currently stored
that way), large_signed() likewise. Writes go through new set_number /
set_large_unsigned / set_large_signed methods that keep the
discriminator and the union value in sync.

This was prompted by an observation that moving json_type to the end
of the object should shrink things via tail-padding reuse. Empirically
the type-at-end rearrangement saves nothing on its own (every
subclass payload is 8-byte aligned so it can't slot into the 4-byte
tail), but the discriminated-number redesign hits the same idea from
a different direction: adding the 4-byte `repr` to json_number makes
the class non-standard-layout, which lets the Itanium ABI pack `repr`
into the base's 4-byte tail padding at offset 20. The union value
then starts at the natural offset 24, and json_number ends at offset
32 -- a 33% reduction.

Per-node sizes:
  json_object (TRUE/FALSE/NULL)  24 bytes
  json_number                    32 bytes  (was 48)
  json_string                    48 bytes
  json_array                     48 bytes
  json_hash                      48 bytes

Numbers dominate real GeoJSON (every coordinate is one), so the net
memory win on a typical parse is substantial.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-30 17:49:24 -07:00
Erica FischerandCursor bd90f0b4fe Move parser-only expect state out of json_object
`expect` was only meaningful while the parser was building a container,
and only ever read or written from jsonpull.cpp itself; once parsing
finished it was dead weight on every JSON_ARRAY and JSON_HASH (and
present-but-unused on every primitive too). Move it into the parser's
container stack, alongside the shared_ptr to the container it pertains
to:

    struct json_pull::parse_frame {
        json_object_ptr container;
        json_type       expect;
    };
    std::vector<parse_frame> container_stack;

The base class now only carries data-model state (parent, parser, type).
No external caller depended on `expect`, so no sweep was needed outside
jsonpull.cpp.

This change does not, in itself, shrink any json_object: the 4-byte
`expect` field used to live at offset 20 inside the base, where it was
already being eaten by alignment padding for the 8-byte-aligned first
member of every subclass (std::string, std::vector, double). The win is
in the data model, not the byte count -- the 4-byte hole is still
there, but it is now available for a future subclass whose first member
is small enough to slot into it.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-30 17:49:19 -07:00
Erica FischerandCursor 4d9a48c3d4 Store hash key/value pairs in one ordered vector
Replace the parallel std::vector<json_object_ptr> keys / values on
json_hash with a single std::vector<json_entry>, where json_entry is
a small {key, value} aggregate. This still preserves insertion order
(the property the parallel vectors were providing) but removes the
"keep two vectors in lockstep" pattern, and call sites can now use
range-for with structured bindings:

    for (auto &[k, v] : o->entries()) { ... }

Side effects:

* sizeof(json_hash) drops from 72 to 48 bytes (one fewer vector
  header), matching json_array.
* The keys() and values() accessors on json_object are replaced by a
  single entries() accessor returning std::vector<json_entry>&.
* All call sites were swept from the old paired-index pattern
  (`o->keys()[i]` / `o->values()[i]`) to entry-based access. Where the
  original pattern relied on `nprop = 0` to short-circuit iteration on
  a null or non-hash `properties`, the rewrite now guards the loop
  explicitly with `if (o->type == JSON_HASH)` so that calling
  entries() doesn't trip the asserting downcast.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-30 17:49:13 -07:00
Erica FischerandCursor f366b2c4aa Subclass json_object so primitives shrink from 168 to 24 bytes
The previous "every member in a struct" layout cost 168 bytes per
json_object, even for JSON_NULL / JSON_TRUE / JSON_FALSE nodes that
have no payload. Splitting json_object into a small base class plus
json_number / json_string / json_array / json_hash subclasses brings
each instance down to just the size of its actual contents:

  json_object (base, TRUE / FALSE / NULL)   24 bytes
  json_number                                48 bytes
  json_string                                48 bytes
  json_array  (empty)                        48 bytes
  json_hash   (empty)                        72 bytes

Other size wins along the way:

* Drop enable_shared_from_this<json_object> (its embedded weak_ptr
  was 16 bytes per node). json_pull now keeps an explicit
  container_stack and the parser no longer needs to resurrect a
  shared_ptr from a raw `parent` walk.
* Remove the unused `refcon` slot from the string variant.
* No virtual destructor: shared_ptr keeps the deleter from the
  original std::make_shared<json_xxx> call, so destroying a
  shared_ptr<json_object> still runs the right subclass dtor.

The base class exposes type-tagged accessors (o->string(),
o->number(), o->array(), o->keys(), o->values(), o->large_signed(),
o->large_unsigned()) that assert the type matches and downcast to
the appropriate subclass storage. All call sites were swept from
the old `o->value.X.Y` field paths to these accessors. A raw-pointer
overload of json_hash_get() replaces the few external uses of
shared_from_this() that survived in geojson-loop.cpp.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-30 17:49:08 -07:00
Erica FischerandCursor 3da03c6075 Convert jsonpull to C++ with shared_ptr and std::vector/std::string
Replace the manual malloc/realloc/free memory management in jsonpull
with std::shared_ptr ownership. Each json_object now owns its children
through std::vector<json_object_ptr>; raw back-pointers to parent and
parser remain valid by structural invariant and are cleared on
json_disconnect so detached subtrees can outlive their parser.
Strings become std::string, child arrays become std::vector, and the
old union becomes a struct so non-trivial members can coexist while
preserving the existing o->value.xxx access paths.

The old jsonpull.c is replaced by jsonpull.cpp, json_stringify now
returns std::string, and all callers across tippecanoe, tile-join,
tippecanoe-decode, tippecanoe-json-tool, tippecanoe-overzoom and the
unit tests are updated to use json_object_ptr / json_pull_ptr.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-30 17:48:32 -07:00
Erica Fischer 9381158165 Clear for merge 2026-05-30 17:48:21 -07:00
Erica Fischer fe7e84be4a Rename to jsonpull.cpp 2026-05-30 17:43:12 -07:00