geojson icon indicating copy to clipboard operation
geojson copied to clipboard

Change serde::Serialize implementations to serialize structures directly without building up a JsonObject

Open frewsxcv opened this issue 5 years ago • 4 comments

Currently, our serde::Serialize implementations build up a JsonObject (a serde_json::Map) that gets serialized by Serde, and then gets thrown away. Building up a JsonObject causes new allocations to happen, including cloning all coordinates.

Instead of building up a JsonObject, we should be able to use the serialization methods provided by serde::Serialize directly. For reference, here's the serde::Serialize implementation for serde_json::Value which may be helpful. In particular, we'll probably want to build up a use serde::ser::SerializeMap.

After it's done, we should benchmark the performance. We currently only have a parsing benchmark, so we should create a new one for writing GeoJSON.

frewsxcv avatar Dec 03 '20 23:12 frewsxcv

Spawned from this discussion: https://github.com/georust/geojson/pull/151#discussion_r535670765

frewsxcv avatar Dec 03 '20 23:12 frewsxcv

I'm interested in looking into this. It looks like flattening the map from Value into Geometry might be a bit tricky, but I can open up an initial PR for feedback on that

pjsier avatar Dec 08 '20 18:12 pjsier

want to build up a use serde::ser::SerializeMap

I'm not very familiar with serde. It seems like this would also be a bunch of allocation? I was hoping for some kind of "visitor" pattern which avoid allocating anything per-geometry, but I don't know what's possible.

michaelkirk avatar Dec 08 '20 21:12 michaelkirk

maybe relevant: https://serde.rs/stream-array.html

michaelkirk avatar Dec 08 '20 21:12 michaelkirk