AER-6605 - upgraded geo (s2-geometry) submodule.

* Added abseil and s2geometry submodules.

---------

Co-authored-by: varun <[email protected]>
Co-authored-by: Sunil Sayyaparaju <[email protected]>

.gitmodules

@@ -12,13 +12,18 @@
 [submodule "modules/luajit"]
 	path = modules/luajit
 	url = https://github.com/aerospike/luajit.git
-[submodule "modules/s2-geometry-library"]
-	path = modules/s2-geometry-library
-	url = https://github.com/aerospike/s2-geometry-library.git
 [submodule "modules/telemetry"]
 	path = modules/telemetry
 	url = https://github.com/aerospike/aerospike-telemetry-agent.git
 [submodule "modules/jemalloc"]
 	path = modules/jemalloc
 	url = https://github.com/aerospike/jemalloc.git
 	ignore = dirty
+[submodule "modules/abseil-cpp"]
+	path = modules/abseil-cpp
+	url = https://github.com/aerospike/abseil-cpp.git
+	ignore = dirty
+[submodule "modules/s2geometry"]
+	path = modules/s2geometry
+	url = https://github.com/aerospike/s2geometry.git
+	ignore = dirty

Makefile

@@ -42,14 +42,14 @@ include make_in/Makefile.vars
 
 .PHONY: all server
 all server: aslibs
-	$(MAKE) -C as
+	$(MAKE) -C as OS=$(OS)
 
 .PHONY: lib
 lib: aslibs
-	$(MAKE) -C as $@ STATIC_LIB=1
+	$(MAKE) -C as $@ STATIC_LIB=1 OS=$(OS)
 
 .PHONY: aslibs
-aslibs: targetdirs version $(JANSSON)/Makefile $(JEMALLOC)/Makefile $(LUAJIT)/src/luaconf.h
+aslibs: targetdirs version $(JANSSON)/Makefile $(JEMALLOC)/Makefile $(LUAJIT)/src/luaconf.h s2lib
 ifeq ($(USE_LUAJIT),1)
 	$(MAKE) -C $(LUAJIT) Q= TARGET_SONAME=libluajit.so CCDEBUG=-g
 endif
@@ -61,8 +61,29 @@ endif
 	$(MAKE) -C $(COMMON) CF=$(CF) EXT_CFLAGS="$(EXT_CFLAGS)" OS=$(UNAME)
 	$(MAKE) -C $(CF)
 	$(MAKE) -C $(MOD_LUA) CF=$(CF) COMMON=$(COMMON) EXT_CFLAGS="$(EXT_CFLAGS)" USE_LUAJIT=$(USE_LUAJIT) LUAJIT=$(LUAJIT) TARGET_SERVER=1 OS=$(UNAME)
-	$(MAKE) -C $(S2)
 
+.PHONY: absllib
+absllib:
+ifeq ($(OS),$(filter $(OS), el7 ubuntu18.04)) # cmake version < 3.13
+	mkdir -p $(ABSL)/build
+	cd $(ABSL)/build && $(CMAKE) -DCMAKE_INSTALL_PREFIX=$(ABSL)/installation -DABSL_ENABLE_INSTALL=ON -DCMAKE_CXX_STANDARD=17 ..
+else
+	$(CMAKE) -S $(ABSL) -B $(ABSL)/build -DCMAKE_INSTALL_PREFIX=$(ABSL)/installation -DABSL_ENABLE_INSTALL=ON -DCMAKE_CXX_STANDARD=17
+endif
+	$(CMAKE) --build $(ABSL)/build
+	$(CMAKE) --build $(ABSL)/build --target install
+	ar rcsT $(ABSL_LIB_DIR)/libabsl.a $(ABSL_LIB_DIR)/libabsl_*.a
+
+.PHONY: s2lib
+s2lib: absllib
+ifeq ($(OS),$(filter $(OS), el7 ubuntu18.04)) # cmake version < 3.13
+	mkdir -p $(S2)/build
+	cd $(S2)/build && $(CMAKE) $(if $(OPENSSL_INCLUDE_DIR),-DOPENSSL_INCLUDE_DIR=$(OPENSSL_INCLUDE_DIR),) -DCMAKE_PREFIX_PATH=$(ABSL)/installation -DBUILD_SHARED_LIBS=OFF -DCMAKE_CXX_STANDARD=17 ..
+else
+	$(CMAKE) -S $(S2) -B $(S2)/build $(if $(OPENSSL_INCLUDE_DIR),-DOPENSSL_INCLUDE_DIR=$(OPENSSL_INCLUDE_DIR),) -DCMAKE_PREFIX_PATH=$(ABSL)/installation -DBUILD_SHARED_LIBS=OFF -DCMAKE_CXX_STANDARD=17
+endif
+	$(CMAKE) --build $(S2)/build -- -j 4 # Limit threads as the default spawns too many
+
 .PHONY: targetdirs
 targetdirs:
 	mkdir -p $(GEN_DIR) $(LIBRARY_DIR) $(BIN_DIR)
@@ -114,7 +135,8 @@ cleanmodules:
 		$(MAKE) -C $(LUAJIT) clean; \
 	fi
 	$(MAKE) -C $(MOD_LUA) COMMON=$(COMMON) USE_LUAJIT=$(USE_LUAJIT) LUAJIT=$(LUAJIT) clean
-	$(MAKE) -C $(S2) clean
+	$(RM) -rf $(ABSL)/build $(ABSL)/installation # ABSL default clean leaves files in build directory
+	$(RM) -rf $(S2)/build # S2 default clean leaves files in build directory
 
 .PHONY: cleandist
 cleandist:

as/include/geospatial/geojson.h

@@ -1,5 +1,5 @@
 /* 
- * Copyright 2015 Aerospike, Inc.
+ * Copyright 2015-2023 Aerospike, Inc.
  *
  * Portions may be licensed to Aerospike, Inc. under one or more
  * contributor license agreements.
@@ -23,8 +23,8 @@
 
 #include <jansson.h>
 
-#include <s2cellid.h>
-#include <s2region.h>
+#include <s2/s2cell_id.h>
+#include <s2/s2region.h>
 
 namespace GeoJSON {
 
@@ -35,7 +35,7 @@ class GeometryHandler
 
 	virtual void handle_point(S2CellId const & cellid);
 
-	virtual bool handle_region(S2Region * regionp);
+	virtual void handle_region(S2Region * i_regionp);
 
 	virtual double earth_radius_meters() {
 		return 6371000.0;		// Wikipedia, mean radius.

as/src/Makefile

@@ -235,7 +235,8 @@ INCLUDES += -I$(CF)/include
 INCLUDES += -I$(COMMON)/target/$(PLATFORM)/include
 INCLUDES += -I$(JANSSON)/src
 INCLUDES += -I$(MOD_LUA)/target/$(PLATFORM)/include
-INCLUDES += -I$(S2)
+INCLUDES += -I$(S2)/src
+INCLUDES += -I$(ABSL)
 
 ifeq ($(USE_LUAJIT),1)
   INCLUDES += -I$(LUAJIT)/src
@@ -285,9 +286,10 @@ else
   LIBRARIES += -L$(JANSSON)/src/.libs -ljansson
 endif
 
-LIBRARIES += -L$(S2) -ls2 -ls2cellid -lgoogle-strings -lgoogle-base \
-			-lgoogle-util-coding -lgoogle-util-math \
-			$(shell curl-config --libs) -lstdc++
+LIBRARIES += $(shell curl-config --libs) -lstdc++
+
+AS_LIBRARIES += $(S2)/build/libs2.a
+AS_LIBRARIES += $(ABSL_LIB_DIR)/libabsl.a
 
 LIBRARIES := $(AS_LIBRARIES) $(LIBRARIES)
 
@@ -320,6 +322,6 @@ $(SERVER): $(OBJECTS) $(AS_LIB_DEPS)
 include $(DEPTH)/make_in/Makefile.targets
 
 # Ignore S2 induced warnings
-S2_WNO = -Wno-unused-local-typedefs -Wno-deprecated -Wno-sign-compare
-$(OBJECT_DIR)/geospatial/%.o: CXXFLAGS += $(S2_WNO) -D__STDC_LIMIT_MACROS
+S2_WNO = -Wno-attributes -Wno-sign-compare -Wno-deprecated-declarations -Wno-class-memaccess
+$(OBJECT_DIR)/geospatial/%.o: CXXFLAGS += $(S2_WNO) -std=c++17 -D__STDC_LIMIT_MACROS
 $(OBJECT_DIR)/geospatial/%.o: CFLAGS := $(filter-out -std=gnu99,$(CFLAGS))

as/src/geospatial/geojson.cc

@@ -1,7 +1,7 @@
 /*
  * geojson.cc
  *
- * Copyright (C) 2015-2020 Aerospike, Inc.
+ * Copyright (C) 2015-2023 Aerospike, Inc.
  *
  * Portions may be licensed to Aerospike, Inc. under one or more contributor
  * license agreements.
@@ -27,12 +27,11 @@
 
 #include <jansson.h>
 
-#include <s2.h>
-#include <s2cap.h>
-#include <s2cellid.h>
-#include <s2polygon.h>
-#include <s2regionunion.h>
-#include <s2latlng.h>
+#include <s2/s2cap.h>
+#include <s2/s2cell_id.h>
+#include <s2/s2polygon.h>
+#include <s2/s2region_union.h>
+#include <s2/s2latlng.h>
 
 #include "geospatial/scoped.h"
 #include "geospatial/throwstream.h"
@@ -91,7 +90,7 @@ traverse_point(json_t * coord)
 	return latlng.ToPoint();
 }
 
-S2Loop *
+unique_ptr<S2Loop>
 traverse_loop(json_t * vertices)
 {
 	if (! vertices) {
@@ -124,12 +123,12 @@ traverse_loop(json_t * vertices)
 	}
 	points.pop_back();
 
-	auto_ptr<S2Loop> loop(new S2Loop(points));
+	unique_ptr<S2Loop> loop(new S2Loop(points));
 	loop->Normalize();
-	return loop.release();
+	return loop;
 }
 
-S2Polygon *
+unique_ptr<S2Polygon>
 traverse_polygon(json_t * loops)
 {
 	if (! loops) {
@@ -140,27 +139,24 @@ traverse_polygon(json_t * loops)
 		throwstream(runtime_error, "polygon body is not array");
 	}
 
-	vector<S2Loop *> loopv;
+	vector<unique_ptr<S2Loop>> loopv;
 	try
 	{
 		for (size_t ii = 0; ii < json_array_size(loops); ++ii) {
 			loopv.push_back(traverse_loop(json_array_get(loops, ii)));
 		}
 
-		return new S2Polygon(&loopv);
+		return make_unique<S2Polygon>(move(loopv));
 	}
 	catch (...)
 	{
-		for (size_t ii = 0; ii < loopv.size(); ++ii) {
-			delete loopv[ii];
-		}
 		throw;
 	}
 }
 
 void process_point(GeoJSON::GeometryHandler & geohand, json_t * coord)
 {
-	geohand.handle_point(S2CellId::FromPoint(traverse_point(coord)));
+	geohand.handle_point(S2CellId(traverse_point(coord)));
 }
 
 void
@@ -174,10 +170,8 @@ process_polygon(GeoJSON::GeometryHandler & geohand, json_t * coord)
 		throwstream(runtime_error, "coordinates are not array");
 	}
 
-	S2Polygon * poly = traverse_polygon(coord);
-	if (geohand.handle_region(poly)) {
-		delete poly;
-	}
+	unique_ptr<S2Polygon> polygonp = traverse_polygon(coord);
+	geohand.handle_region(polygonp.release());
 }
 
 void
@@ -191,16 +185,13 @@ process_multipolygon(GeoJSON::GeometryHandler & geohand, json_t * coord)
 		throwstream(runtime_error, "coordinates are not array");
 	}
 
-	auto_ptr<S2RegionUnion> regionsp(new S2RegionUnion);
+	unique_ptr<S2RegionUnion> regionsp(new S2RegionUnion);
 
 	for (size_t ii = 0; ii < json_array_size(coord); ++ii) {
 		regionsp->Add(traverse_polygon(json_array_get(coord, ii)));
 	}
 
-	if (! geohand.handle_region(regionsp.get())) {
-		// Handler took ownership.
-		regionsp.release();
-	}
+	geohand.handle_region(regionsp.release());
 }
 
 void
@@ -239,12 +230,9 @@ process_circle(GeoJSON::GeometryHandler & geohand, json_t * coord)
 
 	S1Angle angle = S1Angle::Radians(radius / geohand.earth_radius_meters());
 
-	auto_ptr<S2Cap> capp(S2Cap::FromAxisAngle(center, angle).Clone());
+	unique_ptr<S2Cap> capp(new S2Cap(center, angle));
 
-	if (! geohand.handle_region(capp.get())) {
-		// Handler took ownership.
-		capp.release();
-	}
+	geohand.handle_region(capp.release());
 }
 
 void traverse_geometry(GeoJSON::GeometryHandler & geohand, json_t * geom)
@@ -293,10 +281,9 @@ void GeometryHandler::handle_point(S2CellId const & i_cellid)
 	// nothing by default
 }
 
-bool GeometryHandler::handle_region(S2Region * i_regionp)
+void GeometryHandler::handle_region(S2Region * i_regionp)
 {
-	// By default, caller should delete the region.
-	return true;
+	// nothing by default
 }
 
 void parse(GeometryHandler & geohand, string const & geostr)

as/src/geospatial/geospatial.cc

@@ -1,7 +1,7 @@
 /*
  * geospatial.cc
  *
- * Copyright (C) 2015-2020 Aerospike, Inc.
+ * Copyright (C) 2015-2023 Aerospike, Inc.
  *
  * Portions may be licensed to Aerospike, Inc. under one or more contributor
  * license agreements.
@@ -26,7 +26,7 @@
 
 #include <stdexcept>
 
-#include <s2regioncoverer.h>
+#include <s2/s2region_coverer.h>
 
 extern "C" {
 #include "log.h"
@@ -54,9 +54,8 @@ class PointRegionHandler: public GeoJSON::GeometryHandler
 		m_cellid = cellid;
 	}
 
-	virtual bool handle_region(S2Region * regionp) {
+	virtual void handle_region(S2Region * regionp) {
 		m_regionp = regionp;
-		return false;	// Don't delete this region, please.
 	}
 
 	virtual double earth_radius_meters() {
@@ -104,27 +103,28 @@ geo_region_cover(const as_namespace * ns,
 		S2Region * regionp = (S2Region *) region;
 
 		S2RegionCoverer coverer;
+		S2RegionCoverer::Options * options = coverer.mutable_options();
 		if (ns) {
-			coverer.set_min_level(ns->geo2dsphere_within_min_level);
-			coverer.set_max_level(ns->geo2dsphere_within_max_level);
-			coverer.set_max_cells(ns->geo2dsphere_within_max_cells);
-			coverer.set_level_mod(ns->geo2dsphere_within_level_mod);
+			options->set_min_level(ns->geo2dsphere_within_min_level);
+			options->set_max_level(ns->geo2dsphere_within_max_level);
+			options->set_max_cells(ns->geo2dsphere_within_max_cells);
+			options->set_level_mod(ns->geo2dsphere_within_level_mod);
 		}
 		else {
 			// FIXME - we really don't want to hardcode these values, but
 			// some callers can't provide the namespace context ...
-			coverer.set_min_level(1);
-			coverer.set_max_level(30);
-			coverer.set_max_cells(12);
-			coverer.set_level_mod(1);
+			options->set_min_level(1);
+			options->set_max_level(30);
+			options->set_max_cells(12);
+			options->set_level_mod(1);
 		}
 		vector<S2CellId> covering;
 		coverer.GetCovering(*regionp, &covering);
 
 		// The coverer can always return 6 cells, even when max cells is
 		// less (regions which intersect all cube faces).  If we get more
 		// then we asked for and it's greater then 6 something is wrong.
-		if (covering.size() > max(size_t(6), size_t(coverer.max_cells()))) {
+		if (covering.size() > max(size_t(6), size_t(options->max_cells()))) {
 			return false;
 		}
 
@@ -207,7 +207,7 @@ geo_point_within(uint64_t cellidval, geo_region_t region)
 	{
 		S2Region * regionp = (S2Region *) region;
 		S2CellId cellid(cellidval);
-		bool iswithin = regionp->VirtualContainsPoint(cellid.ToPoint());
+		bool iswithin = regionp->Contains(cellid.ToPoint());
 		return iswithin;
 	}
 	catch (exception const & ex)