Http tiles #1887

gknisely · 2019-07-18T00:17:42Z

This PR does 3 things:

it allows caching of tiles to disk that have been fetched via curl, previously the cache was in memory so when the cache was flushed the tiles would need to be fetched via curl again if another access was needed
it adds a method, given a bounding box, to expand or contract the bounding box to fit the edges which have begin nodes within the bounding box
it adds remove_all support to filesystem as its needed for proper testing of caching tiles to disk

both things have unit tests though the second one has its own executable as well, which will of course tank the coverage report.

src/mjolnir/valhalla_expand_bounding_box.cc

src/loki/route_action.cc

src/mjolnir/valhalla_add_predicted_traffic.cc

src/baldr/graphtile.cc

CMakeLists.txt

kevinkreiser · 2019-07-25T18:12:52Z

src/baldr/graphreader.cc

@@ -630,5 +645,43 @@ std::unordered_set<GraphId> GraphReader::GetTileSet(const uint8_t level) const {
  return tiles;
 }

+AABB2<PointLL> GraphReader::GetMinimumBoundingBox(const AABB2<PointLL>& bb) {


this is one of the main areas of new functionality. will give you a right sized bounding box that will look at the input box, find all the nodes in it and expand/contract the bounding box to fit the edges leaving those nodes and their shape

kevinkreiser · 2019-07-25T18:13:40Z

src/baldr/graphtile.cc


-      // for setting where to read compressed data from


i moved this into its own function so we can call it from multiple code paths, one such code path is seen above

kevinkreiser · 2019-07-25T18:14:04Z

src/baldr/graphtile.cc

+    // and if we cant cache to disk try to use it in-memory
+    auto tile = GraphTile();
+    if (gzipped) {
+      tile.DecompressTile(graphid, tile_data);


heres the other spot where this is called

kevinkreiser · 2019-07-25T18:17:00Z

src/baldr/graphtile.cc

@@ -126,24 +129,51 @@ GraphTile::GraphTile(const GraphId& graphid, char* ptr, size_t size) : header_(n
  Initialize(graphid, ptr, size);
 }

-GraphTile::GraphTile(const std::string& tile_url, const GraphId& graphid, curler_t& curler) {
+GraphTile GraphTile::CacheTileURL(const std::string& tile_url,


made this into a static function that returns a graphtile instead of a constructor. its actually probably not necessary to be honest but i thought because of the caching to disk function it was a bit less tricky or more obvious to give it a name. let me know if anyone thinks this is better off as a constructor.

kevinkreiser · 2019-07-25T18:17:50Z

src/baldr/graphtile.cc

-    Initialize(graphid, graphtile_->data(), graphtile_->size());
-    // TODO: optionally write the tile to disk?
+    // try to cache it on disk so we dont have to keep doing this
+    if (!cache_location.empty()) {


the caching part is optional though, so if you dont specify a tile_dir in the valhalla config it will just use the in memory cache as it did before with urls

kevinkreiser · 2019-07-25T18:19:16Z

src/baldr/graphtile.cc

+      tile.DecompressTile(graphid, tile_data);
+    } // we dont need to decompress so just take ownership of the data
+    else {
+      tile.graphtile_.reset(new std::vector<char>(0, 0));


we also support non gzipped tiles, there is a new config parameter to tell which one to expect when curling. the reason you want this is so that you can cache a gzipped tile on disk isntead of always unpacking everything and storing uncompressed data on disk.

kevinkreiser · 2019-07-25T18:19:48Z

src/baldr/graphtile.cc

@@ -303,7 +333,7 @@ void GraphTile::AssociateOneStopIds(const GraphId& graphid) {
  }
 }

-std::string GraphTile::FileSuffix(const GraphId& graphid) {
+std::string GraphTile::FileSuffix(const GraphId& graphid, bool gzipped) {


when making file names its convenient to have this flag

kevinkreiser · 2019-07-25T18:20:10Z

src/midgard/point2.cc

 namespace valhalla {
 namespace midgard {

-bool Point2::ApproximatelyEqual(const Point2& p) const {
-  return equal<first_type>(first, p.first, EPSILON) && equal<second_type>(second, p.second, EPSILON);
+bool Point2::ApproximatelyEqual(const Point2& p, float e) const {


for testing its nice to be able to specify the required precision

kevinkreiser · 2019-07-25T18:20:47Z

src/valhalla_expand_bounding_box.cc

@@ -0,0 +1,112 @@
+#include "baldr/graphreader.h"


this is the program that lets you get bounding boxes on the command line. this will be unfortunate for the coverage report

kevinkreiser · 2019-07-25T18:22:01Z

test/http_tiles.cc

@@ -0,0 +1,224 @@
+#include "test.h"


this is an ingenous little test if i do say so myself :) it runs an http file server using prime_server to serve up the utrecht tiles we use for lots of other unit tests.

kevinkreiser · 2019-07-25T18:23:31Z

test/http_tiles.cc

+    std::fstream input(full_path, std::ios::in | std::ios::binary);
+    if (input) {
+      std::string buffer((std::istreambuf_iterator<char>(input)), std::istreambuf_iterator<char>());
+      if (gz)


the cool thing is, that it does gzip compression of the responses on the fly so that when graphreader comes at it with a .gz request it returns a gzipped payload with proper content encoding. otherwise it returns the raw bytes from disk. this lets us test all the options below

kevinkreiser · 2019-07-25T18:27:15Z

test/minbb.cc

@@ -0,0 +1,97 @@
+#include "test.h"


here we test the minimum bounding box functionality by first configuring graphreader to nothing and seeing its an invalid bb and then configure it to utrecht. we test both expansion and contraction.

kevinkreiser · 2019-07-25T18:27:43Z

valhalla/baldr/graphtile.h

+   * @param  node  GraphId of the node from which the transitions leave
+   * @return returns an iterable collection of node transitions
+   */
+  midgard::iterable_t<const NodeInfo> GetNodes() const {


this was just convenient so i added it

kevinkreiser · 2019-07-25T18:28:53Z

valhalla/filesystem.h

@@ -187,16 +189,47 @@ class directory_entry {
  filesystem::path path_;
 };

+class directory_iterator {


to do recursive delete, we need a non-recursive directory iterator, go figure. the recursion happens inside of the delete not at the iterator level

kevinkreiser · 2019-07-25T18:29:29Z

valhalla/filesystem.h

@@ -308,4 +341,19 @@ inline bool remove(const path& p) {
  return ::remove(p.c_str()) == 0;
 }

+inline bool remove_all(const path& p) {


heres where we implement rm -r. i think the comments suitably explain what is going on

…cache again. unshadow variable

dnesbitt61 · 2019-07-26T11:46:21Z

src/baldr/graphreader.cc

      return nullptr;
    }
+    // LOG_DEBUG("Memory map cache hit " + GraphTile::FileSuffix(base));


did you intend to leave in these log messages?

in practice they were too chatty. i left them because they are useful for debugging this specific thing but i thought they were too much to always have on. trace would probably be more appropriate

dnesbitt61 · 2019-07-26T11:50:36Z

src/baldr/graphreader.cc

+        if (!min_bb.minpt().IsValid())
+          min_bb = AABB2<PointLL>(node_ll, node_ll);
+
+        // Look at the shape of each edge leaving the node


do you really need the shape? can you just get the end node and use it to expand the bounding box? The shape will be held in the tile of the end node. Tough I guess if you want to do a loki search near a part of the shape outside both the begin and end node's tile then you would want to include the tile of the shape point (since that tile will have the bins to index into the end node tiles).

kevinkreiser · 2019-07-26T16:39:42Z

src/baldr/graphtile.cc

    if (!cache_location.empty()) {
      auto disk_location = cache_location + filesystem::path::preferred_separator + suffix;
      auto dir = filesystem::path(cache_location + filesystem::path::preferred_separator + suffix);
      dir.replace_filename("");
      if (filesystem::create_directories(dir)) {
        std::ofstream file(disk_location, std::ios::out | std::ios::binary | std::ios::ate);
        file.write(&tile_data[0], tile_data.size());
-        return GraphTile(cache_location, graphid);


this was a bug and a slower way of doing it. it was a bug because we didnt call file.close() before teling the constructor to go read it off the disk. it was slow because we already have the bytes in memory from curl and so we just load them directly below

kevinkreiser · 2019-07-26T17:21:21Z

going to update the changelog and merge

kevinkreiser and others added 9 commits July 16, 2019 16:07

cache url fetched tiles to disk

6772d02

if we fail to write to disk we can still use it in memory

08c2942

Merge remote-tracking branch 'origin/master' into http_tiles

ec4da36

clean up.

10d0c6f

reader not used

896074b

added valhalla_expand_bounding_box

ebaa2d3

init cp. Finds all the nodes in the bounding box and then expands

41ac381

clean up

a81e33c

Merge branch 'master' into http_tiles

f886249