Optimise encodePolyline function (#6940)

CHANGELOG.md

@@ -24,6 +24,7 @@
     - NodeJS:
       - CHANGED: Use node-api instead of NAN. [#6452](https://github.com/Project-OSRM/osrm-backend/pull/6452)
     - Misc:
+      - CHANGED: Optimise encodePolyline function. [#6940](https://github.com/Project-OSRM/osrm-backend/pull/6940)
       - CHANGED: Avoid reallocations in base64 encoding. [#6951](https://github.com/Project-OSRM/osrm-backend/pull/6951)
       - CHANGED: Get rid of unused Boost dependencies. [#6960](https://github.com/Project-OSRM/osrm-backend/pull/6960)
       - CHANGED: Apply micro-optimisation for Table & Trip APIs. [#6949](https://github.com/Project-OSRM/osrm-backend/pull/6949)

include/engine/polyline_compressor.hpp

@@ -12,7 +12,7 @@ namespace osrm::engine
 {
 namespace detail
 {
-std::string encode(std::vector<int> &numbers);
+void encode(int number_to_encode, std::string &output);
 std::int32_t decode_polyline_integer(std::string::const_iterator &first,
                                      std::string::const_iterator last);
 } // namespace detail
@@ -30,27 +30,24 @@ std::string encodePolyline(CoordVectorForwardIter begin, CoordVectorForwardIter
         return {};
     }
 
-    std::vector<int> delta_numbers;
+    std::string output;
-    BOOST_ASSERT(size > 0);
+    // just a guess that we will need ~4 bytes per coordinate to avoid reallocations
-    delta_numbers.reserve((size - 1) * 2);
+    output.reserve(size * 4);
+
     int current_lat = 0;
     int current_lon = 0;
-    std::for_each(
+    for (auto it = begin; it != end; ++it)
-        begin,
+    {
-        end,
+        const int lat_diff =
-        [&delta_numbers, &current_lat, &current_lon, coordinate_to_polyline](
+            std::round(static_cast<int>(it->lat) * coordinate_to_polyline) - current_lat;
-            const util::Coordinate loc)
+        const int lon_diff =
-        {
+            std::round(static_cast<int>(it->lon) * coordinate_to_polyline) - current_lon;
-            const int lat_diff =
+        detail::encode(lat_diff, output);
-                std::round(static_cast<int>(loc.lat) * coordinate_to_polyline) - current_lat;
+        detail::encode(lon_diff, output);
-            const int lon_diff =
+        current_lat += lat_diff;
-                std::round(static_cast<int>(loc.lon) * coordinate_to_polyline) - current_lon;
+        current_lon += lon_diff;
-            delta_numbers.emplace_back(lat_diff);
+    }
-            delta_numbers.emplace_back(lon_diff);
+    return output;
-            current_lat += lat_diff;
-            current_lon += lon_diff;
-        });
-    return detail::encode(delta_numbers);
 }
 
 // Decodes geometry from polyline format

src/engine/polyline_compressor.cpp

@@ -10,9 +10,19 @@
 namespace osrm::engine::detail // anonymous to keep TU local
 {
 
-std::string encode(int number_to_encode)
+void encode(int number_to_encode, std::string &output)
 {
-    std::string output;
+    if (number_to_encode < 0)
+    {
+        const unsigned binary = std::llabs(number_to_encode);
+        const unsigned twos = (~binary) + 1u;
+        const unsigned shl = twos << 1u;
+        number_to_encode = static_cast<int>(~shl);
+    }
+    else
+    {
+        number_to_encode <<= 1u;
+    }
     while (number_to_encode >= 0x20)
     {
         const int next_value = (0x20 | (number_to_encode & 0x1f)) + 63;
@@ -22,31 +32,6 @@ std::string encode(int number_to_encode)
 
     number_to_encode += 63;
     output += static_cast<char>(number_to_encode);
-    return output;
-}
-
-std::string encode(std::vector<int> &numbers)
-{
-    std::string output;
-    for (auto &number : numbers)
-    {
-        if (number < 0)
-        {
-            const unsigned binary = std::llabs(number);
-            const unsigned twos = (~binary) + 1u;
-            const unsigned shl = twos << 1u;
-            number = static_cast<int>(~shl);
-        }
-        else
-        {
-            number <<= 1u;
-        }
-    }
-    for (const int number : numbers)
-    {
-        output += encode(number);
-    }
-    return output;
 }
 
 // https://developers.google.com/maps/documentation/utilities/polylinealgorithm

unit_tests/engine/polyline_compressor.cpp

@@ -45,4 +45,96 @@ BOOST_AUTO_TEST_CASE(polyline6_test_case)
         decodePolyline<1000000>(encodePolyline<1000000>(coords.begin(), coords.end())).begin()));
 }
 
+BOOST_AUTO_TEST_CASE(empty_polyline_test)
+{
+    using namespace osrm::engine;
+    using namespace osrm::util;
+
+    std::vector<Coordinate> empty_coords;
+    BOOST_CHECK_EQUAL(encodePolyline(empty_coords.begin(), empty_coords.end()), "");
+    BOOST_CHECK(decodePolyline("").empty());
+}
+BOOST_AUTO_TEST_CASE(polyline_single_point_test)
+{
+    using namespace osrm::engine;
+    using namespace osrm::util;
+
+    const std::vector<Coordinate> coords({{FixedLongitude{-122414000}, FixedLatitude{37776000}}});
+
+    const std::string encoded = encodePolyline(coords.begin(), coords.end());
+    BOOST_CHECK_EQUAL(encoded, "_cqeFn~cjV");
+
+    const auto decoded = decodePolyline(encoded);
+    BOOST_CHECK_EQUAL(decoded.size(), 1);
+    BOOST_CHECK_EQUAL(decoded[0].lon, FixedLongitude{-122414000});
+    BOOST_CHECK_EQUAL(decoded[0].lat, FixedLatitude{37776000});
+}
+
+BOOST_AUTO_TEST_CASE(polyline_multiple_points_test)
+{
+    using namespace osrm::engine;
+    using namespace osrm::util;
+
+    const std::vector<Coordinate> coords({{FixedLongitude{-122414000}, FixedLatitude{37776000}},
+                                          {FixedLongitude{-122420000}, FixedLatitude{37779000}},
+                                          {FixedLongitude{-122421000}, FixedLatitude{37780000}}});
+
+    const std::string encoded = encodePolyline(coords.begin(), coords.end());
+    BOOST_CHECK_EQUAL(encoded, "_cqeFn~cjVwQnd@gEfE");
+
+    const auto decoded = decodePolyline(encoded);
+    BOOST_CHECK_EQUAL(decoded.size(), 3);
+    for (size_t i = 0; i < coords.size(); ++i)
+    {
+        BOOST_CHECK_EQUAL(decoded[i].lon, coords[i].lon);
+        BOOST_CHECK_EQUAL(decoded[i].lat, coords[i].lat);
+    }
+}
+
+BOOST_AUTO_TEST_CASE(polyline_large_coordinate_difference_test)
+{
+    using namespace osrm::engine;
+    using namespace osrm::util;
+
+    const std::vector<Coordinate> coords({{FixedLongitude{-179000000}, FixedLatitude{-89000000}},
+                                          {FixedLongitude{179000000}, FixedLatitude{89000000}}});
+
+    const std::string encoded = encodePolyline(coords.begin(), coords.end());
+    BOOST_CHECK_EQUAL(encoded, "~xe~O~|oca@_sl}`@_{`hcA");
+
+    const auto decoded = decodePolyline(encoded);
+    BOOST_CHECK_EQUAL(decoded.size(), 2);
+    for (size_t i = 0; i < coords.size(); ++i)
+    {
+        BOOST_CHECK_EQUAL(decoded[i].lon, coords[i].lon);
+        BOOST_CHECK_EQUAL(decoded[i].lat, coords[i].lat);
+    }
+}
+
+BOOST_AUTO_TEST_CASE(roundtrip)
+{
+    using namespace osrm::engine;
+    using namespace osrm::util;
+
+    {
+        const auto encoded = "_chxEn`zvN\\\\]]";
+        const auto decoded = decodePolyline(encoded);
+        const auto reencoded = encodePolyline(decoded.begin(), decoded.end());
+        BOOST_CHECK_EQUAL(encoded, reencoded);
+    }
+    {
+        const auto encoded =
+            "gcneIpgxzRcDnBoBlEHzKjBbHlG`@`IkDxIiKhKoMaLwTwHeIqHuAyGXeB~Ew@fFjAtIzExF";
+        const auto decoded = decodePolyline(encoded);
+        const auto reencoded = encodePolyline(decoded.begin(), decoded.end());
+        BOOST_CHECK_EQUAL(encoded, reencoded);
+    }
+    {
+        const auto encoded = "_p~iF~ps|U_ulLnnqC_mqNvxq`@";
+        const auto decoded = decodePolyline(encoded);
+        const auto reencoded = encodePolyline(decoded.begin(), decoded.end());
+        BOOST_CHECK_EQUAL(encoded, reencoded);
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()