Read a KML Placemark's geometry from the Placemark down

[njakobsen]

Resolves #66.

each_record read a geometry's name and metadata from feature.ancestors('Placemark').first. Nokogiri::XML::Node#ancestors answers a selector by walking to the document root and searching the whole document from there, then scanning the results for each ancestor, and it caches nothing. Called once per geometry, that costs geometries x document size.

A KML where each Placemark holds one geometry never shows it. One exported from design software does: a Placemark is a <MultiGeometry> of hundreds of small faces, so a few thousand Placemarks become six figures of geometries in a document tens of megabytes wide.

The iteration is inverted rather than optimised in place. each_record starts at each Placemark and reads the geometry below it, so the Placemark's metadata, name and image paths are read once and shared by its parts, and no geometry ever asks what it belongs to.

What each change is worth

Measured on a 33.8 MB CAD-derived KML from a production deployment, holding 76,190 geometries in 1,233 Placemarks, with the per-geometry database round trip stubbed out so the figures cover the traversal alone:

	Traversal
ancestors + metadata per geometry (master)	902.02s
Parent walk + metadata per geometry (first commit)	10.76s
Placemark-first (this branch)	2.86s

Eliminating the upward search is worth 891.3s. Eliminating the repeated extract_metadata is worth a further 7.9s, which is most of what remained after the first change but small against the original cost. The inversion removes the question rather than making it cheaper to ask.

The remaining cost of this import is the per-geometry ST_GeomFromKML round trip, which this PR does not touch.

Geometry outside a Placemark

Iterating Placemarks alone silently drops geometry that sits outside one, which the importer supports and kmz_file_features_without_placemarks.kmz covers. A second pass matches it with a single XPath over the document, //Polygon[not(ancestor::Placemark)] and the same for the other two types, rather than by asking each element for its ancestors.

That fixture is not passing vacuously: removing the second pass fails kml_file_spec.rb:26, which asserts the count. The two sibling examples in that shared group assert blank names and empty metadata, both of which hold against an empty collection, so the count is the only guard among the three.

Two hazards the inversion introduces, both covered

Image paths. images_from_metadata deletes the key it reads. Reading it once per part would leave every part after the first without images. It is now read once per Placemark and shared. kml_file_with_multi_geometry_photos.kml covers it, and fails against the straightforward port that calls it per part.

Shared metadata. Each part takes its own copy, since one hash behind several records is one object several Features would write through.

Iteration order changes

It was every Polygon in the document, then every LineString, then every Point. It is now each Placemark's geometry together in document order, then any geometry outside a Placemark.

Nothing in the suite asserts on feature sequence; the assertions are count, all, and contain_exactly. features_hash derives from Importers::Base#cache_key, which is Digest::MD5.hexdigest(@data) over the source bytes rather than over what the importer emits, so regrouping cannot invalidate it. mvt_sql applies order(:id) at query time. Feature ids do land in a different order.

A Placemark inside another Placemark

Reading a Placemark's geometry with placemark.css(...) matches everything below it, including the geometry of a Placemark nested inside it. Both are iterated, so that geometry would be read twice where the nearest-ancestor lookup read it once. KML 2.2 does not allow the nesting — a Placemark is a Feature, and Features nest only inside Containers — but a document that does it should not import duplicates.

geometries_in scopes the match to elements whose nearest enclosing Placemark is that one. XPath 1.0 has no node-identity operator, so the scope is expressed by depth rather than by asking whether an ancestor is a particular node.

The scoped path counts each candidate's ancestors, which is an upward walk per element and the class of work this PR removes. Three approaches over 76,190 geometries in 1,233 Placemarks, on a 32 MB document:

	Time	Elements
css('Polygon, LineString, Point')	0.045s	76,190
Depth-scoped path, every Placemark	0.330s	76,190
Detect nesting once, then css	0.051s	76,190

All three return the same elements. //Placemark//Placemark answers once per document whether anything nests, at 0.021s, so a document that does not nest keeps the plain selector and the scoped path runs only where it is needed.

kml_file_with_nested_placemarks.kml covers it: reading it yields two features, and yields three when the scoping is removed.

Fixtures

Adds kml_file_with_multi_geometry_placemarks.kml, kml_file_with_multi_geometry_photos.kml and kml_file_with_nested_placemarks.kml, all built by fixtures:build from the same round-degree squares as the other KML fixtures, with generic layer and placemark names. None of them is derived from any real file. multi_geometry is factored out of the first builder for the second, and regenerating leaves the first byte-identical.

Verification

Full suite locally: 312 examples, 0 failures, 19 pending.

Each guard was checked by breaking it: reverting the call site fails the no-further-search example, removing the unplaced-geometry pass fails the without-placemarks count, moving images_from_metadata back inside the per-part loop fails the photos example, and dropping the nesting scope turns the nested-placemark count from two into three.

The importer's output over four real source files, 136,769 features, digests identically before and after the nesting scope was added: same feature count, same stream of geometry, names, metadata and image paths, same warnings.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv

[njakobsen]

We should consider memoizing the placemarks. If we're running this for each feature, there's no reason to derive the same metadata each time if we share placemarks across many features. Let's check if this helps processing times.

[njakobsen]

Memoizing turned out not to be needed, because your next comment removes the thing that would have been memoized. Reading each Placemark's geometry from the Placemark down means the metadata is derived once at the top of the loop and shared by that Placemark's parts, so there is nothing to cache across iterations and no cache to size or invalidate.

I did measure what the repeated derivation was costing, since it is worth knowing whether that alone would have been enough. On a 33.8 MB file holding 76,190 geometries in 1,233 Placemarks, with the database round trip stubbed out:

	Traversal
ancestors + metadata per geometry	902.02s
Parent walk + metadata per geometry	10.76s
Placemark-first	2.86s

So the repeated extract_metadata was 7.9s of the 902s. Memoizing it on its own would have saved that and left the 891s. It is most of what remained after the traversal was fixed, which is why it is still worth having, but it was never the problem on its own.

One thing the sharing needed care with: images_from_metadata deletes the key it reads, so calling it per part would give the images to the first part and nothing to the rest. It is now called once per Placemark alongside extract_metadata. kml_file_with_multi_geometry_photos.kml covers that, and fails against the version that calls it per part.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv

[njakobsen]

Perhaps instead of doing this how we are right now, we should start at each placemark and search for the kml types from that node downwards. That would avoid the need to memoize more than one placemark's metadata at a time, and would eliminate the need to search up the node tree.

[njakobsen]

Done — this is what the branch now does. each_record iterates Placemarks, reads metadata, name and image paths once each, then reads the geometry below with placemark.css('Polygon, LineString, Point'). enclosing_placemark is gone, and so is the {'Polygon' => 'POLYGON', ...} hash whose sql_type half was unused.

Measured on a 33.8 MB file holding 76,190 geometries in 1,233 Placemarks, database round trip stubbed out, the traversal goes from 902.02s on master to 2.86s.

Two things it needed beyond the straight inversion:

Geometry outside a Placemark. Iterating Placemarks alone drops it silently, and kmz_file_features_without_placemarks.kmz covers that case. There is now a second pass, one XPath over the document (//Polygon[not(ancestor::Placemark)] and the same for the other two types), rather than a per-element ancestor question. Removing that pass fails kml_file_spec.rb:26, so it is a real guard rather than a vacuous one.

Image paths. images_from_metadata removes the key it reads, so it had to move up beside extract_metadata rather than staying per-part. Covered by a new fixture.

Iteration order changes as a result: each Placemark's geometry together in document order, rather than all Polygons then all LineStrings then all Points document-wide. I checked what depends on it — nothing in the suite asserts sequence, mvt_sql orders by id at query time, and features_hash is an MD5 of the source bytes rather than of the emitted features, so it cannot be invalidated by regrouping. Feature ids do land in a different order. Details in the PR body.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv

[njakobsen]

The nested-Placemark case now has a guard rather than a note saying KML forbids the nesting.

A selector can express it, but not directly: XPath 1.0 has no node-identity operator, so "elements whose nearest Placemark ancestor is this node" cannot be asked for. Scoping by depth gets the same answer — count how many Placemarks a candidate sits inside and keep the ones at this Placemark's depth plus one.

The cost is the reason it is not used unconditionally. That predicate walks upward once per candidate, which is the work this branch removes. Measured over 76,190 geometries in 1,233 Placemarks on a 32 MB document:

	Time	Elements
css('Polygon, LineString, Point')	0.045s	76,190
Depth-scoped path, every Placemark	0.330s	76,190
Detect nesting once, then css	0.051s	76,190

All three return the same elements. The scoped path is 7.4x the plain selector, so //Placemark//Placemark answers once per document whether anything nests — 0.021s on that document — and the scoped path runs only where it is needed. A well-formed document keeps the plain selector and pays only the detection.

kml_file_with_nested_placemarks.kml covers it, built by fixtures:build from the same squares as the other KML fixtures. Reading it yields two features, and yields three when the scoping is removed.

The importer's output over four real source files, 136,769 features, digests identically before and after: same count, same stream of geometry, names, metadata and image paths, same warnings. Full suite 312 examples, 0 failures.

#70 is rebased onto this and its suite is green at 317 examples.

🤖 Generated with Claude Code

https://claude.ai/code/session_012b7n8NbLAd6s7c3atii4Mv