Replaces the shared_ptr-based json_object_ptr with a unique_ptr that
has a stateless custom deleter dispatching on json_object::type before
calling the right subclass destructor. Eliminates per-node atomic
reference-counting and the control-block allocation that shared_ptr
required for every node in the tree.
API now distinguishes owning and borrowing pointers explicitly:
- json_read / json_read_separators / json_hash_get return raw
json_object * (borrowed from the parser-owned tree).
- json_read_tree / json_disconnect return json_object_ptr (caller
takes ownership; back-pointers are cleared so the subtree can
outlive the parser).
- json_free / json_context / json_stringify take raw pointers.
- The parser's container_stack holds raw pointers; jp->root keeps
unique_ptr ownership of the most recent top-level value.
Internally, take_from_owner moves the unique_ptr out of whichever
parent vector / hash entry / parser root owned it, which both
json_free and json_disconnect rely on.
In the streaming parsers (parse_feature, parse_layers, the
geojson-loop callback), we are careful to free `j` only after we
have processed a complete Feature: json_read returns each token
as the tree is being built up, and freeing an intermediate node
would splice it out of the surrounding hash and corrupt the
in-progress feature.
Benchmark (tl_2022_us_county.json, -z0 --extend-zooms-if-still-dropping,
median of 5 runs on macOS arm64): 8.5s, vs 10.6s with shared_ptr
and 8.7s on the pre-refactor C baseline.
Co-authored-by: Cursor <cursoragent@cursor.com>
In the C++ port, json_free was just `o.reset()`, which dropped the
caller's reference but left the subtree alive: the parent's vector
slot kept it allocated, and for line-delimited streams the parser's
jp->root co-owned it until the next top-level value started parsing.
That defeated the geojson-loop pattern of calling json_free on each
feature after serializing it, which is supposed to release the
feature so it doesn't sit in memory while subsequent ones are parsed.
Restore the historical "remove this from the tree" semantics by
splicing the node out of its parent (sharing splice_from_parent with
json_disconnect) and clearing jp->root when the node is the parser's
current top-level value, then dropping the caller's reference.
Two unit tests pin this down: a pruning test parses
"[[1, 2], [3, 4], [5, 6]]" element-wise and confirms that calling
json_free on [3, 4] leaves the outer array with just [1, 2] and
[5, 6]; a top-level test uses a weak_ptr observer to confirm that
json_free on the parser's root really destroys the tree.
Co-authored-by: Cursor <cursoragent@cursor.com>
Profiling tl_2022_us_county.json (sample(1) on Apple Silicon) showed
~38% of parse time in allocator work and ~14% in std::string::push_back
during string-token construction. These changes target the low-hanging
fruit from that profile:
- Pre-reserve 2 slots in json_array and 4 slots in json_hash so
coordinate `[x, y]` pairs and typical GeoJSON property maps avoid
the 0 -> 1 -> 2 -> 4 vector-growth chain (and the shared_ptr copies
it incurs).
- Reuse a parser-wide std::string buffer for JSON_STRING tokens
instead of constructing a fresh local std::string per token. The
buffer is cleared (capacity preserved) at the start of each token
and copied into the final json_string, so once it has grown to the
longest string seen it stops reallocating entirely.
- std::move the freshly-created container shared_ptr into the parser
container stack in the `[` and `{` handlers, and move it out of the
frame on the matching `]` / `}`. Each move skips one atomic
inc/dec round-trip per container open and close.
On a tl_2022_us_county.json benchmark (4-iter user-time mean, Apple
Silicon, /usr/bin/time):
- main baseline: ~8.17s
- jsonpull-cpp before these changes: ~10.90s (+33%)
- jsonpull-cpp with these changes: ~9.33s (+14%)
So this commit recovers roughly half of the post-port regression.
The remaining gap is dominated by shared_ptr atomic refcount traffic
on the parse tree and per-node heap allocations, which would require
the larger unique_ptr/arena reworks to address.
Co-authored-by: Cursor <cursoragent@cursor.com>
Covers the `c` vs `ch` bug fixed in the previous commit: parsing
"\uD83D\uE000" (a valid high surrogate followed by a non-surrogate
BMP code point) used to mis-classify U+E000 as a low surrogate and
combine the two units into U+1F400 (F0 9F 90 80). The fixed code
flushes the stale high surrogate as standalone CESU-8 (ED A0 BD)
and then encodes U+E000 normally as EE 80 80. Verified the test
fails under the pre-fix logic.
Co-authored-by: Cursor <cursoragent@cursor.com>
- 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>
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>
`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>
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>
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>
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>
Enhance fqsort function to accept a temporary directory parameter for file handling. Update calls to fqsort in main.cpp, sort.cpp, sort.hpp, and unit.cpp to utilize the new parameter, ensuring temporary files are created in the specified directory.
* Divide-and-conquer polygon cleaning
* Catch the case where the gap can't be increased further
* Catch the case where we try to keep impossibly many features
* Make label points earlier in the tiling process
* Another case where it could try to drop even after already limiting.
* And do not coalesce on impossibly small geometries
* Add missing return
* Update version and changelog
* Improve the memory spike during tile construction
* Remove `need_tilestats`. Add a bunch of debug logging
* Remove debug logging
* Update version and changelog
* Add a flag to use an H3 index for the feature index
* Give mvt_value and serial_val a double-with-count concept
* Switch mean over to internal accumulation state
* Get rid of the attribute accumulation map
* Change vectors of features to vectors of pointers to features
* Fix --coalesce
* Revert "Add a flag to use an H3 index for the feature index"
This reverts commit b9b48f42c9.
* Update version and changelog
* Sketching out sqlite options for tile-join
* Enable sqlite3 serialized multithreading
* Fix some unnecessary round trips from std::string to char * and back
* Gathering join keys for sql query
* Make a query
* Actually open the gpkg. Fix the query quoting.
* Actually do the query and get results back
* Join the attributes onto the feature
* Set matched if the sql join matches
* Add a flag to get the feature ID from the query
* Observe attribute exclusion when joining from sql queries
* Add a flag to exclude all attributes from the tile side of the join
* Make tile-join bounding boxes reflect feature bounds, not tile bounds
* More tests
* An empty tileset has empty bounds at null island
* Gather the results from each thread *after* the thread finishes
* Missed a test
* Fix accidental inclusion of the top left of the tile in the bbox
* Case smashing and prefix trimming in the select
* Add test of sql join
* Make the join column option a join expression option
* Fix antimeridian adjustment. Z0 can't wait until the end of the tile
* Checkpoint on accepting multiple joined rows per tiled feature
* Adding the joined attribute should be per-feature, not per-attribute
* Forgot to update the test. Order of joined attributes has changed.
* Get the attributes back in the right order
* Add test of sql join with limit
* Allow multiple tile features to have the same join key
* Update tests for a country name with two distinct geometries
* Update version and changelog
* Forgot to mention the bounding box improvements
* Make tippecanoe-overzoom accept filters from a file
* Accept clip polygons from a file too
* Add test of clipping by polygon from file
* Add a test of reading an overzoom filter from a file
* Update version and changelog
* Plumb a clip bounding box around through overzoom
* Actually do some clipping
* Add a test
* Fix post-binning clipping
* Factoring out geometry parsing from feature parsing
* Accept a clip polygon argument to tippecanoe-overzoom
* Progress in the direction of polygon clipping
* Fix the wagyu flags. We need intersection, not union
* Remove debug spew
* Clip points to polygon bounds too
* Copy the geometric binning code to serve as intersection-finding code
* Add clipper2 for linestring clipping
* Compiles, but does not actually seem to clip. Hmm.
* Oh, it helps if I actually call the function
* Add clipping tests
* Add missing fixture, and don't crash if it is missing
* Remember to do polygon clipping after binning too
* Fix scaling before post-binning clipping. Add test.
* Remove unused parts of clipper
* Rename for consistency
* Revert accidentally added line
* Clip the clip regions to the tile bounds to reduce their complexity
* Add a test of clipping the clip region down to the tile boundary
* Update version and changelog
* Strip out unwanted attributes earlier in the process
* Skip aggregations whose attributes have been excluded
* Forgot one
* Add some more tests
* Update version and changelog
* Add a tippecanoe-decode option to restrict which attributes to decode
* Plumb buffer and feature limit around
* Check the feature limit
* Clarifying cases where output detail can be unspecified
* Clip bins to the tile buffer instead of just passing them through
* Add missing include
* Missed some tests
* Add --no-tile-compression option to tippecanoe-overzoom
* Update version and changelog
* Progress on plumbing a string pool for full_keys through
* More plumbing for key_pool
* Don't keep features with identical locations as multiplier features
* Revert "Don't keep features with identical locations as multiplier features"
This reverts commit 413f0c8024.
* Adjust calculated maxzoom to account for duplicate feature locations
* Update changelog and version
* Add a test affected by the maxzoom change with duplicate locations
* Round the drop rate a little for cross-platform test consistency
* Add an all-mvt_value attribute accumulation path
* Only bin by ID, not geometrically
* A little cleanup; changelog and version; test
* Remove accidental double-conversion
* Replace duplicated code with template
* Update changelog
* Choose the megatile features from those that will be in the next N zooms
* Take fractional zooms into account in multiplier feature choices
* Fix more tests
* Add a flag to retain multiplier features by minimum distance
* Limit feature expansion from multiplier density to 2x
* The multiplier cap was a bad idea
* Revert "The multiplier cap was a bad idea"
This reverts commit 6f8273a4c8.
* Revert "Limit feature expansion from multiplier density to 2x"
This reverts commit a26e41309d.
* Revert "Add a flag to retain multiplier features by minimum distance"
This reverts commit 01f14a4255.
* Remove the multiplier sequence, which should no longer matter
* Revert "Revert "Add a flag to retain multiplier features by minimum distance""
This reverts commit 776da4a1b8.
* Revert "Revert "Limit feature expansion from multiplier density to 2x""
This reverts commit 44a683d808.
* Revert "Revert "The multiplier cap was a bad idea""
This reverts commit 80f7cb1c0e.
* Track two kinds of previous index for next_feature
* Fix multiplier density threshold, I think
* Oh, I didn't git add the code changes
* Update version and changelog
* Try to install sqlite3 to fix the automated build
* Deleted too much
* Only let --preserve-point-density-threshold shift density around
* Remove the density debt concept, since it doesn't help
* Make the drop states a vector instead of an array
* Revert "Make the drop states a vector instead of an array"
This reverts commit 66c7abb6fa.
* Revert "Remove the density debt concept, since it doesn't help"
This reverts commit 707bb0c562.
* Revert "Only let --preserve-point-density-threshold shift density around"
This reverts commit ecf01f2231.
Erica Fischer