search

georust / wkt

Rust read/write support for well-known text (WKT)
https://crates.io/crates/wkt
Apache License 2.0
51 stars 25 forks source link

Use ryu for faster float-to-string conversion #118

Open kylebarron opened 1 hour ago

kylebarron commented 1 hour ago

In https://github.com/geoarrow/geoarrow-rs/pull/788 @b4l prototyped a custom implementation of writing to WKT, and he said

It is up to twice as fast as wkt and geo.

It doesn't look like this wkt crate uses ryu, and so I'm curious if that's a large part of why his implementation is faster. https://github.com/dtolnay/ryu seems to be a very popular library, and so it doesn't seem like an unreasonable dependency.

It looks like it should be relatively easy to swap in ryu and test if that's significantly faster? It looks like https://github.com/georust/wkt/blob/c3088cdc730c4d643b8535513ddcc3b0efca5e95/src/types/coord.rs#L37-L44 is the main place where floats are converted to strings.

I suppose the primary issue with using ryu is that ryu's Float trait supports only f32 and f64, while WktNum supports a broader range of types, including integers.

Assuming that ryu is indeed quite a bit faster, I think it's worth special casing f32 and f64 if we can, assuming that most real world data will be f64.

(For maintainability reasons, if geoarrow-rs can use wkt directly, that's much more appealing to me than rolling our own).

Any thoughts?

michaelkirk commented 1 hour ago

I think that sounds really promising. I’d prefer not to lose support for integer geometries though. I wonder if there’s a way to accommodate both?On Sep 24, 2024, at 19:22, Kyle Barron @.***> wrote: In geoarrow/geoarrow-rs#788 @b4l prototyped a custom implementation of writing to WKT, and he said

It is up to twice as fast as wkt and geo.

It doesn't look like this wkt crate uses ryu, and so I'm curious if that's a large part of why his implementation is faster. https://github.com/dtolnay/ryu seems to be a very popular library, and so it doesn't seem like an unreasonable dependency. It looks like it should be relatively easy to swap in ryu and test if that's significantly faster? It looks like https://github.com/georust/wkt/blob/c3088cdc730c4d643b8535513ddcc3b0efca5e95/src/types/coord.rs#L37-L44 is the main place where floats are converted to strings. I suppose the primary issue with using ryu is that ryu supports only f32 and f64, while WktNum supports a broader range of types, including integers. Assuming that ryu is indeed quite a bit faster, I think it's worth special casing f32 and f64 if we can, assuming that most real world data will be f64. (For maintainability reasons, if geoarrow-rs can use wkt directly, that's much more appealing to me than rolling our own). Any thoughts?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

lnicola commented 1 hour ago

This was discussed a little in https://github.com/rust-lang/rust/issues/52811.