From 7d72dfebf7750f3f64eba9596a2c8df1ddaf750a Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sat, 22 Jun 2024 20:04:32 +0200 Subject: [PATCH] Optimise encodePolyline function (#6940) --- CHANGELOG.md | 1 + include/engine/polyline_compressor.hpp | 37 +++++---- src/engine/polyline_compressor.cpp | 39 +++------- unit_tests/engine/polyline_compressor.cpp | 92 +++++++++++++++++++++++ 4 files changed, 122 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e7ef6891..79a234ec9 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/include/engine/polyline_compressor.hpp b/include/engine/polyline_compressor.hpp index b2a505764..af58b0422 100644 --- a/include/engine/polyline_compressor.hpp +++ b/include/engine/polyline_compressor.hpp @@ -12,7 +12,7 @@ namespace osrm::engine { namespace detail { -std::string encode(std::vector &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 delta_numbers; - BOOST_ASSERT(size > 0); - delta_numbers.reserve((size - 1) * 2); + std::string output; + // just a guess that we will need ~4 bytes per coordinate to avoid reallocations + output.reserve(size * 4); + int current_lat = 0; int current_lon = 0; - std::for_each( - begin, - end, - [&delta_numbers, ¤t_lat, ¤t_lon, coordinate_to_polyline]( - const util::Coordinate loc) - { - const int lat_diff = - std::round(static_cast(loc.lat) * coordinate_to_polyline) - current_lat; - const int lon_diff = - std::round(static_cast(loc.lon) * coordinate_to_polyline) - current_lon; - delta_numbers.emplace_back(lat_diff); - delta_numbers.emplace_back(lon_diff); - current_lat += lat_diff; - current_lon += lon_diff; - }); - return detail::encode(delta_numbers); + for (auto it = begin; it != end; ++it) + { + const int lat_diff = + std::round(static_cast(it->lat) * coordinate_to_polyline) - current_lat; + const int lon_diff = + std::round(static_cast(it->lon) * coordinate_to_polyline) - current_lon; + detail::encode(lat_diff, output); + detail::encode(lon_diff, output); + current_lat += lat_diff; + current_lon += lon_diff; + } + return output; } // Decodes geometry from polyline format diff --git a/src/engine/polyline_compressor.cpp b/src/engine/polyline_compressor.cpp index 3d4500643..7435fed3f 100644 --- a/src/engine/polyline_compressor.cpp +++ b/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(~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(number_to_encode); - return output; -} - -std::string encode(std::vector &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(~shl); - } - else - { - number <<= 1u; - } - } - for (const int number : numbers) - { - output += encode(number); - } - return output; } // https://developers.google.com/maps/documentation/utilities/polylinealgorithm diff --git a/unit_tests/engine/polyline_compressor.cpp b/unit_tests/engine/polyline_compressor.cpp index fa05512a9..e2a9a5fd7 100644 --- a/unit_tests/engine/polyline_compressor.cpp +++ b/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 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 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 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 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()