some memory and concurrency improvements

.gitignore

@@ -17,3 +17,4 @@ include/vector_tile.pb.h
 # downloaded data
 coastline
 landcover
+/test.*

CMakeLists.txt

@@ -87,33 +87,32 @@ ADD_CUSTOM_COMMAND(OUTPUT osmformat.pb.cc osmformat.pb.h
 
 file(GLOB tilemaker_src_files
 	src/attribute_store.cpp
-	src/geom.cpp
-	src/mbtiles.cpp
-	src/osm_mem_tiles.cpp
-	src/output_object.cpp
-	src/read_pbf.cpp
-	src/shared_data.cpp
-	src/tile_data.cpp
-	src/tile_worker.cpp
 	src/coordinates.cpp
+	src/coordinates_geom.cpp
+	src/external/streamvbyte_decode.cc
+	src/external/streamvbyte_encode.cc
+	src/external/streamvbyte_zigzag.cc
+	src/geom.cpp
 	src/helpers.cpp
+	src/mbtiles.cpp
+	src/mmap_allocator.cpp
+	src/node_stores.cpp
 	src/osm_lua_processing.cpp
+	src/osm_mem_tiles.cpp
 	src/osm_store.cpp
-	src/mmap_allocator.cpp
+	src/output_object.cpp
 	src/pbf_blocks.cpp
+	src/read_pbf.cpp
 	src/read_shp.cpp
+	src/shared_data.cpp
 	src/shp_mem_tiles.cpp
-	src/tilemaker.cpp
-	src/write_geometry.cpp
-	src/node_stores.cpp
-	src/coordinates_geom.cpp
 	src/sorted_node_store.cpp
 	src/sorted_way_store.cpp
+	src/tile_data.cpp
+	src/tilemaker.cpp
+	src/tile_worker.cpp
 	src/way_stores.cpp
-	src/external/streamvbyte_decode.cc
-	src/external/streamvbyte_encode.cc
-	src/external/streamvbyte_zigzag.cc
-
+	src/write_geometry.cpp
   )
 add_executable(tilemaker vector_tile.pb.cc osmformat.pb.cc ${tilemaker_src_files})
 target_include_directories(tilemaker PRIVATE include)

Makefile

@@ -133,7 +133,7 @@ test_sorted_way_store: \
 	src/mmap_allocator.o \
 	src/sorted_way_store.o \
 	src/sorted_way_store.test.o
-	$(CXX) $(CXXFLAGS) -o test $^ $(INC) $(LIB) $(LDFLAGS) && ./test
+	$(CXX) $(CXXFLAGS) -o test.sorted_way_store $^ $(INC) $(LIB) $(LDFLAGS) && ./test.sorted_way_store
 
 
 %.o: %.cpp

include/attribute_store.h

@@ -145,9 +145,8 @@ struct AttributePair {
 // It should be at least 2x the number of your cores -- 256 shards is probably
 // reasonable for most people.
 //
-// We also reserve the bottom shard for the hot pool. Since a shard is 16M entries,
-// but the hot pool is only 64KB entries, we're wasting a little bit of key space.
-#define SHARD_BITS 8
+// We also reserve the bottom shard for the hot pool.
+#define SHARD_BITS 14
 #define ATTRIBUTE_SHARDS (1 << SHARD_BITS)
 
 class AttributePairStore {

include/clip_cache.h

@@ -0,0 +1,79 @@
+#ifndef _CLIP_CACHE_H
+#define _CLIP_CACHE_H
+
+#include "coordinates.h"
+#include "coordinates_geom.h"
+#include "geom.h"
+#include <mutex>
+
+class TileBbox;
+
+template <class T>
+class ClipCache {
+public:
+	ClipCache(size_t threadNum, unsigned int baseZoom):
+		baseZoom(baseZoom),
+		clipCache(threadNum * 16),
+		clipCacheMutex(threadNum * 16),
+		clipCacheSize(threadNum * 16) {
+	}
+
+	const std::shared_ptr<T> get(uint zoom, TileCoordinate x, TileCoordinate y, NodeID objectID) const{
+		// Look for a previously clipped version at z-1, z-2, ...
+
+		std::lock_guard<std::mutex> lock(clipCacheMutex[objectID % clipCacheMutex.size()]);
+		while (zoom > 0) {
+			zoom--;
+			x /= 2;
+			y /= 2;
+			const auto& cache = clipCache[objectID % clipCache.size()];
+			const auto& rv = cache.find(std::make_tuple(zoom, TileCoordinates(x, y), objectID));
+			if (rv != cache.end()) {
+				return rv->second;
+			}
+		}
+
+		return nullptr;
+	}
+
+	void add(const TileBbox& bbox, const NodeID objectID, const T& output) {
+		// The point of caching is to reuse the clip, so caching at the terminal zoom is
+		// pointless.
+		if (bbox.zoom == baseZoom)
+			return;
+
+		std::shared_ptr<T> copy = std::make_shared<T>();
+		boost::geometry::assign(*copy, output);
+
+		size_t index = objectID % clipCacheMutex.size();
+		std::vector<std::shared_ptr<T>> objects;
+		std::lock_guard<std::mutex> lock(clipCacheMutex[index]);
+		auto& cache = clipCache[index];
+		// Reset the cache periodically so it doesn't grow without bound.
+		//
+		// I also tried boost's lru_cache -- but it seemed to perform worse, maybe
+		// due to the bookkeeping? We could try authoring a bounded map that
+		// evicts in FIFO order, which will have less bookkeeping.
+		clipCacheSize[index]++;
+		if (clipCacheSize[index] > 1024) {
+			clipCacheSize[index] = 0;
+			// Copy the map's contents to a vector so that calling .clear()
+			// and releasing the lock can happen separately from running the
+			// destructors of all of the objects.
+			objects.reserve(1025);
+			for (const auto& x : cache)
+				objects.push_back(x.second);
+			cache.clear();
+		}
+
+		cache[std::make_tuple(bbox.zoom, bbox.index, objectID)] = copy;
+	}
+
+private:
+	unsigned int baseZoom;
+	std::vector<std::map<std::tuple<uint16_t, TileCoordinates, NodeID>, std::shared_ptr<T>>> clipCache;
+	mutable std::vector<std::mutex> clipCacheMutex;
+	std::vector<size_t> clipCacheSize;
+};
+
+#endif

include/geometry_cache.h

@@ -0,0 +1,53 @@
+#ifndef _GEOMETRY_CACHE_H
+#define _GEOMETRY_CACHE_H
+
+// A small class to map from ID -> geometry, with
+// a bounded size and LRU-ish behaviour.
+//
+// Not safe for multi-threaded use!
+
+#include "coordinates.h"
+#include <memory>
+
+#define NUM_BUCKETS 256
+// Keep bucket size small so linear search is fast
+#define BUCKET_SIZE 32
+
+
+template <class T>
+class GeometryCache {
+public:
+	GeometryCache():
+		offsets(NUM_BUCKETS),
+		ids(NUM_BUCKETS * BUCKET_SIZE),
+		geometries(NUM_BUCKETS * BUCKET_SIZE) {}
+
+	T* get(NodeID objectID) {
+		const size_t bucket = objectID % NUM_BUCKETS;
+
+		for (size_t i = bucket * BUCKET_SIZE; i < (bucket + 1) * BUCKET_SIZE; i++)
+			if (ids[i] == objectID)
+				return geometries[i].get();
+
+		return nullptr;
+	}
+
+	void add(NodeID objectID, std::shared_ptr<T> geom) {
+		const size_t bucket = objectID % NUM_BUCKETS;
+		size_t offset = bucket * BUCKET_SIZE + offsets[bucket];
+		geometries[offset] = geom;
+		ids[offset] = objectID;
+		offsets[bucket]++;
+
+		if (offsets[bucket] == BUCKET_SIZE)
+			offsets[bucket] = 0;
+	}
+
+private:
+	std::vector<uint8_t> offsets;
+	std::vector<NodeID> ids;
+	// CONSIDER: we could use a raw pointer - getOrBuildLinestring controls the lifetime
+	std::vector<std::shared_ptr<Linestring>> geometries;
+};
+
+#endif

include/leased_store.h

@@ -0,0 +1,82 @@
+#ifndef _LEASED_STORE_H
+#define _LEASED_STORE_H
+
+#include "tile_data.h"
+
+// LeasedStore implements "leases" for generated geometries.
+//
+// When Lua code calls Layer(...), we need to store the point, linestring,
+// multilinestring, polygon geometry.
+//
+// Previously, we had code that would lock on each insertion:
+//
+// lock_guard<mutex> lock(mutex);
+// insert(...)
+//
+// Instead, store leases let each of N threads claim a range of 1/N of the IDs.
+// Then each thread can insert into their own stores without taking additional
+// locks.
+
+template<typename S>
+std::vector<std::pair<size_t, S*>>& getAvailableLeases(TileDataSource* source) {
+	throw std::runtime_error("you need to specialize this");
+}
+
+template<>
+inline std::vector<std::pair<size_t, TileDataSource::point_store_t*>>& getAvailableLeases(TileDataSource* source) {
+	return source->availablePointStoreLeases;
+}
+
+template<>
+inline std::vector<std::pair<size_t, TileDataSource::linestring_store_t*>>& getAvailableLeases(TileDataSource* source) {
+	return source->availableLinestringStoreLeases;
+}
+
+template<>
+inline std::vector<std::pair<size_t, TileDataSource::multi_linestring_store_t*>>& getAvailableLeases(TileDataSource* source) {
+	return source->availableMultiLinestringStoreLeases;
+}
+
+template<>
+inline std::vector<std::pair<size_t, TileDataSource::multi_polygon_store_t*>>& getAvailableLeases(TileDataSource* source) {
+	return source->availableMultiPolygonStoreLeases;
+}
+
+
+template <class T>
+class LeasedStore {
+	std::vector<std::pair<TileDataSource*, std::pair<size_t, T*>>> leases;
+
+public:
+	~LeasedStore() {
+		for (const auto& lease : leases) {
+			auto source = lease.first;
+			std::lock_guard<std::mutex> lock(source->storeMutex);
+
+			std::vector<std::pair<size_t, T*>>& availableLeases = getAvailableLeases<T>(source);
+			availableLeases.push_back(lease.second);
+		}
+	}
+
+	std::pair<size_t, T*> get(TileDataSource* source) {
+		for (const auto& lease : leases) {
+			if (lease.first == source)
+				return lease.second;
+		}
+
+		std::lock_guard<std::mutex> lock(source->storeMutex);
+
+		std::vector<std::pair<size_t, T*>>& availableLeases = getAvailableLeases<T>(source);
+
+		if (availableLeases.empty())
+			throw std::runtime_error("fatal: no available stores to lease");
+
+		std::pair<size_t, T*> entry = availableLeases.back();
+		availableLeases.pop_back();
+
+		leases.push_back(std::make_pair(source, entry));
+		return entry;
+	}
+
+};
+#endif

include/osm_lua_processing.h

@@ -44,12 +44,15 @@ class OsmLuaProcessing {
 	// ----	initialization routines
 
 	OsmLuaProcessing(
-        OSMStore &osmStore,
-        const class Config &configIn, class LayerDefinition &layers, 
+		OSMStore &osmStore,
+		const class Config &configIn,
+		class LayerDefinition &layers, 
 		const std::string &luaFile,
 		const class ShpMemTiles &shpMemTiles, 
 		class OsmMemTiles &osmMemTiles,
-		AttributeStore &attributeStore);
+		AttributeStore &attributeStore,
+		bool materializeGeometries
+	);
 	~OsmLuaProcessing();
 
 	// ----	Helpers provided for main routine
@@ -77,7 +80,7 @@ class OsmLuaProcessing {
 	void setNode(NodeID id, LatpLon node, const tag_map_t &tags);
 
 	/// \brief We are now processing a way
-	void setWay(WayID wayId, LatpLonVec const &llVec, const tag_map_t &tags);
+	bool setWay(WayID wayId, LatpLonVec const &llVec, const tag_map_t &tags);
 
 	/** \brief We are now processing a relation
 	 * (note that we store relations as ways with artificial IDs, and that
@@ -123,32 +126,34 @@ class OsmLuaProcessing {
 	std::vector<double> Centroid();
 	Point calculateCentroid();
 
+	enum class CorrectGeometryResult: char { Invalid = 0, Valid = 1, Corrected = 2 };
 	// ----	Requests from Lua to write this way/node to a vector tile's Layer
-    template<class GeometryT>
-    bool CorrectGeometry(GeometryT &geom)
-    {
+	template<class GeometryT>
+	CorrectGeometryResult CorrectGeometry(GeometryT &geom)
+	{
 #if BOOST_VERSION >= 105800
-        geom::validity_failure_type failure = geom::validity_failure_type::no_failure;
-        if (isRelation && !geom::is_valid(geom,failure)) {
-            if (verbose) std::cout << "Relation " << originalOsmID << " has " << boost_validity_error(failure) << std::endl;
-        } else if (isWay && !geom::is_valid(geom,failure)) {
-            if (verbose && failure!=22) std::cout << "Way " << originalOsmID << " has " << boost_validity_error(failure) << std::endl;
-        }
+		geom::validity_failure_type failure = geom::validity_failure_type::no_failure;
+		if (isRelation && !geom::is_valid(geom,failure)) {
+			if (verbose) std::cout << "Relation " << originalOsmID << " has " << boost_validity_error(failure) << std::endl;
+		} else if (isWay && !geom::is_valid(geom,failure)) {
+			if (verbose && failure!=22) std::cout << "Way " << originalOsmID << " has " << boost_validity_error(failure) << std::endl;
+		}
 
 		if (failure==boost::geometry::failure_spikes)
 			geom::remove_spikes(geom);
-        if (failure == boost::geometry::failure_few_points) 
-			return false;
+		if (failure == boost::geometry::failure_few_points) 
+			return CorrectGeometryResult::Invalid;
 		if (failure) {
 			std::time_t start = std::time(0);
 			make_valid(geom);
 			if (verbose && std::time(0)-start>3) {
 				std::cout << (isRelation ? "Relation " : "Way ") << originalOsmID << " took " << (std::time(0)-start) << " seconds to correct" << std::endl;
 			}
+			return CorrectGeometryResult::Corrected;
 		}
 #endif
-		return true;
-    }
+		return CorrectGeometryResult::Valid;
+	}
 
 	// Add layer
 	void Layer(const std::string &layerName, bool area);
@@ -256,6 +261,9 @@ class OsmLuaProcessing {
 	const boost::container::flat_map<std::string, std::string>* currentTags;
 
 	std::vector<OutputObject> finalizeOutputs();
+
+	bool materializeGeometries;
+	bool wayEmitted;
 };
 
 #endif //_OSM_LUA_PROCESSING_H