search

georust / geojson

Library for serializing the GeoJSON vector GIS file format
https://crates.io/crates/geojson
Apache License 2.0
276 stars 60 forks source link

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

Open frewsxcv opened 3 years ago

frewsxcv commented 3 years ago

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 commented 3 years ago

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

pjsier commented 3 years ago

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

michaelkirk commented 3 years ago

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 commented 3 years ago

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