remove cloning from geo iterators in trait (#278)

* wip: remove cloning from geo iterators in trait * On reference as well * Remove clone on geo access * add from_geo bench
geoarrow · Nov 28, 2023 · 9a0df7f · 9a0df7f
1 parent 774b212
commit 9a0df7f
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 57 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -63,6 +63,10 @@ name = "gdal"
 test = false
 required-features = ["gdal"]
 
+[[bench]]
+name = "from_geo"
+harness = false
+
 [[bench]]
 name = "geos_buffer"
 harness = false

diff --git a/benches/from_geo.rs b/benches/from_geo.rs
@@ -0,0 +1,34 @@
+use geo::polygon;
+
+use criterion::{criterion_group, criterion_main, Criterion};
+use geoarrow::array::{MutablePolygonArray, PolygonArray};
+
+fn create_data() -> Vec<geo::Polygon> {
+    // An L shape
+    // https://github.com/georust/geo/blob/7cb7d0ffa6bf1544c5ca9922bd06100c36f815d7/README.md?plain=1#L40
+    let poly = polygon![
+        (x: 0.0, y: 0.0),
+        (x: 4.0, y: 0.0),
+        (x: 4.0, y: 1.0),
+        (x: 1.0, y: 1.0),
+        (x: 1.0, y: 4.0),
+        (x: 0.0, y: 4.0),
+        (x: 0.0, y: 0.0),
+    ];
+    let v = vec![poly; 1000];
+    v
+}
+
+pub fn criterion_benchmark(c: &mut Criterion) {
+    let data = create_data();
+
+    c.bench_function("convert Vec<geo::Polygon> to PolygonArray", |b| {
+        b.iter(|| {
+            let mut_arr = MutablePolygonArray::<i32>::from_polygons(&data, Default::default());
+            let _arr: PolygonArray<i32> = mut_arr.into();
+        })
+    });
+}
+
+criterion_group!(benches, criterion_benchmark);
+criterion_main!(benches);
diff --git a/src/geo_traits/geometry.rs b/src/geo_traits/geometry.rs
@@ -110,3 +110,41 @@ impl<'a, T: CoordNum + 'a> GeometryTrait for Geometry<T> {
         }
     }
 }
+
+impl<'a, T: CoordNum + 'a> GeometryTrait for &'a Geometry<T> {
+    type T = T;
+    type Point<'b> = Point<Self::T> where Self: 'b;
+    type LineString<'b> = LineString<Self::T> where Self: 'b;
+    type Polygon<'b> = Polygon<Self::T> where Self: 'b;
+    type MultiPoint<'b> = MultiPoint<Self::T> where Self: 'b;
+    type MultiLineString<'b> = MultiLineString<Self::T> where Self: 'b;
+    type MultiPolygon<'b> = MultiPolygon<Self::T> where Self: 'b;
+    type GeometryCollection<'b> = GeometryCollection<Self::T> where Self: 'b;
+    type Rect<'b> = Rect<Self::T> where Self: 'b;
+
+    fn as_type(
+        &self,
+    ) -> GeometryType<
+        '_,
+        Point<T>,
+        LineString<T>,
+        Polygon<T>,
+        MultiPoint<T>,
+        MultiLineString<T>,
+        MultiPolygon<T>,
+        GeometryCollection<T>,
+        Rect<T>,
+    > {
+        match self {
+            Geometry::Point(p) => GeometryType::Point(p),
+            Geometry::LineString(p) => GeometryType::LineString(p),
+            Geometry::Polygon(p) => GeometryType::Polygon(p),
+            Geometry::MultiPoint(p) => GeometryType::MultiPoint(p),
+            Geometry::MultiLineString(p) => GeometryType::MultiLineString(p),
+            Geometry::MultiPolygon(p) => GeometryType::MultiPolygon(p),
+            Geometry::GeometryCollection(p) => GeometryType::GeometryCollection(p),
+            Geometry::Rect(p) => GeometryType::Rect(p),
+            _ => todo!(),
+        }
+    }
+}
diff --git a/src/geo_traits/geometry_collection.rs b/src/geo_traits/geometry_collection.rs
@@ -1,6 +1,5 @@
 use super::GeometryTrait;
 use geo::{CoordNum, Geometry, GeometryCollection};
-use std::iter::Cloned;
 use std::slice::Iter;
 
 pub trait GeometryCollectionTrait {
@@ -25,41 +24,41 @@ pub trait GeometryCollectionTrait {
 
 impl<T: CoordNum> GeometryCollectionTrait for GeometryCollection<T> {
     type T = T;
-    type ItemType<'a> = Geometry<Self::T>
+    type ItemType<'a> = &'a Geometry<Self::T>
     where
         Self: 'a;
-    type Iter<'a> = Cloned<Iter<'a, Self::ItemType<'a>>>
+    type Iter<'a> = Iter<'a, Geometry<Self::T>>
     where T: 'a;
 
     fn geometries(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_geometries(&self) -> usize {
         self.0.len()
     }
 
     fn geometry(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
 
 impl<'a, T: CoordNum> GeometryCollectionTrait for &'a GeometryCollection<T> {
     type T = T;
-    type ItemType<'b> = Geometry<Self::T>     where
+    type ItemType<'b> = &'a Geometry<Self::T> where
         Self: 'b;
-    type Iter<'b> = Cloned<Iter<'a, Self::ItemType<'a>>> where
+    type Iter<'b> = Iter<'a, Geometry<Self::T>> where
         Self: 'b;
 
     fn geometries(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_geometries(&self) -> usize {
         self.0.len()
     }
 
     fn geometry(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
diff --git a/src/geo_traits/line_string.rs b/src/geo_traits/line_string.rs
@@ -1,6 +1,5 @@
 use super::CoordTrait;
 use geo::{Coord, CoordNum, LineString};
-use std::iter::Cloned;
 use std::slice::Iter;
 
 pub trait LineStringTrait {
@@ -25,37 +24,36 @@ pub trait LineStringTrait {
 
 impl<T: CoordNum> LineStringTrait for LineString<T> {
     type T = T;
-    type ItemType<'a> = Coord<Self::T> where Self: 'a;
-    type Iter<'a> = Cloned<Iter<'a, Self::ItemType<'a>>> where T: 'a;
+    type ItemType<'a> = &'a Coord<Self::T> where Self: 'a;
+    type Iter<'a> = Iter<'a, Coord<Self::T>> where T: 'a;
 
     fn coords(&self) -> Self::Iter<'_> {
-        // TODO: remove cloned
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_coords(&self) -> usize {
         self.0.len()
     }
 
     fn coord(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
 
 impl<'a, T: CoordNum> LineStringTrait for &'a LineString<T> {
     type T = T;
-    type ItemType<'b> = Coord<Self::T> where Self: 'b;
-    type Iter<'b> = Cloned<Iter<'a, Self::ItemType<'a>>> where Self: 'b;
+    type ItemType<'b> = &'a Coord<Self::T> where Self: 'b;
+    type Iter<'b> = Iter<'a, Coord<Self::T>> where Self: 'b;
 
     fn coords(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_coords(&self) -> usize {
         self.0.len()
     }
 
     fn coord(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
diff --git a/src/geo_traits/multi_line_string.rs b/src/geo_traits/multi_line_string.rs
@@ -1,6 +1,5 @@
 use super::line_string::LineStringTrait;
 use geo::{CoordNum, LineString, MultiLineString};
-use std::iter::Cloned;
 use std::slice::Iter;
 
 pub trait MultiLineStringTrait {
@@ -25,36 +24,36 @@ pub trait MultiLineStringTrait {
 
 impl<T: CoordNum> MultiLineStringTrait for MultiLineString<T> {
     type T = T;
-    type ItemType<'a> = LineString<Self::T> where Self: 'a;
-    type Iter<'a> = Cloned<Iter<'a, Self::ItemType<'a>>> where T: 'a;
+    type ItemType<'a> = &'a LineString<Self::T> where Self: 'a;
+    type Iter<'a> = Iter<'a, LineString<Self::T>> where T: 'a;
 
     fn lines(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_lines(&self) -> usize {
         self.0.len()
     }
 
     fn line(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
 
 impl<'a, T: CoordNum> MultiLineStringTrait for &'a MultiLineString<T> {
     type T = T;
-    type ItemType<'b> = LineString<Self::T> where Self: 'b;
-    type Iter<'b> = Cloned<Iter<'a, Self::ItemType<'a>>> where Self: 'b;
+    type ItemType<'b> = &'a LineString<Self::T> where Self: 'b;
+    type Iter<'b> = Iter<'a, LineString<Self::T>> where Self: 'b;
 
     fn lines(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_lines(&self) -> usize {
         self.0.len()
     }
 
     fn line(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
diff --git a/src/geo_traits/multi_point.rs b/src/geo_traits/multi_point.rs
@@ -1,6 +1,5 @@
 use super::point::PointTrait;
 use geo::{CoordNum, MultiPoint, Point};
-use std::iter::Cloned;
 use std::slice::Iter;
 
 pub trait MultiPointTrait {
@@ -25,36 +24,36 @@ pub trait MultiPointTrait {
 
 impl<T: CoordNum> MultiPointTrait for MultiPoint<T> {
     type T = T;
-    type ItemType<'a> = Point<Self::T> where Self: 'a;
-    type Iter<'a> = Cloned<Iter<'a, Self::ItemType<'a>>> where T: 'a;
+    type ItemType<'a> = &'a Point<Self::T> where Self: 'a;
+    type Iter<'a> = Iter<'a, Point<Self::T>> where T: 'a;
 
     fn points(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_points(&self) -> usize {
         self.0.len()
     }
 
     fn point(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
 
 impl<'a, T: CoordNum> MultiPointTrait for &'a MultiPoint<T> {
     type T = T;
-    type ItemType<'b> = Point<Self::T> where Self: 'b;
-    type Iter<'b> = Cloned<Iter<'a, Self::ItemType<'a>>> where Self: 'b;
+    type ItemType<'b> = &'a Point<Self::T> where Self: 'b;
+    type Iter<'b> = Iter<'a, Point<Self::T>> where Self: 'b;
 
     fn points(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_points(&self) -> usize {
         self.0.len()
     }
 
     fn point(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
diff --git a/src/geo_traits/multi_polygon.rs b/src/geo_traits/multi_polygon.rs
@@ -1,6 +1,5 @@
 use super::polygon::PolygonTrait;
 use geo::{CoordNum, MultiPolygon, Polygon};
-use std::iter::Cloned;
 use std::slice::Iter;
 
 pub trait MultiPolygonTrait {
@@ -25,36 +24,36 @@ pub trait MultiPolygonTrait {
 
 impl<T: CoordNum> MultiPolygonTrait for MultiPolygon<T> {
     type T = T;
-    type ItemType<'a> = Polygon<Self::T> where Self: 'a;
-    type Iter<'a> = Cloned<Iter<'a, Self::ItemType<'a>>> where T: 'a;
+    type ItemType<'a> = &'a Polygon<Self::T> where Self: 'a;
+    type Iter<'a> = Iter<'a, Polygon<Self::T>> where T: 'a;
 
     fn polygons(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_polygons(&self) -> usize {
         self.0.len()
     }
 
     fn polygon(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }
 
 impl<'a, T: CoordNum> MultiPolygonTrait for &'a MultiPolygon<T> {
     type T = T;
-    type ItemType<'b> = Polygon<Self::T> where Self: 'b;
-    type Iter<'b> = Cloned<Iter<'a, Self::ItemType<'a>>> where Self: 'b;
+    type ItemType<'b> = &'a Polygon<Self::T> where Self: 'b;
+    type Iter<'b> = Iter<'a, Polygon<Self::T>> where Self: 'b;
 
     fn polygons(&self) -> Self::Iter<'_> {
-        self.0.iter().cloned()
+        self.0.iter()
     }
 
     fn num_polygons(&self) -> usize {
         self.0.len()
     }
 
     fn polygon(&self, i: usize) -> Option<Self::ItemType<'_>> {
-        self.0.get(i).cloned()
+        self.0.get(i)
     }
 }