Merge branch 'master' into sf-static-tree-prior-queue

2024-06-25 17:51:54 +02:00 · 2024-06-25 17:51:54 +02:00 · 16ea09eeeb
commit 16ea09eeeb
parent 3d9964a1de 5a48ce85b3
9 changed files with 257 additions and 62 deletions
--- a/.github/workflows/osrm-backend.yml
+++ b/.github/workflows/osrm-backend.yml
@ -723,14 +723,34 @@ jobs:
          make -j$(nproc) benchmarks
          cd ..
          make -C test/data 
+      # we run benchmarks in tmpfs to avoid impact of disk IO
+      - name: Create folder for tmpfs 
+        run: mkdir -p /opt/benchmarks
      - name: Run PR Benchmarks 
        run: |
-          ./pr/scripts/ci/run_benchmarks.sh -f $(pwd)/pr -r $(pwd)/pr_results -s $(pwd)/pr -b $(pwd)/pr/build -o ~/data.osm.pbf -g ~/gps_traces.csv
+          sudo mount -t tmpfs -o size=4g none /opt/benchmarks
+          cp -rf pr/build /opt/benchmarks/build
+          mkdir -p /opt/benchmarks/test
+          cp -rf pr/test/data /opt/benchmarks/test/data
+          cp -rf pr/profiles /opt/benchmarks/profiles
+
+          ./pr/scripts/ci/run_benchmarks.sh -f /opt/benchmarks -r $(pwd)/pr_results -s $(pwd)/pr -b /opt/benchmarks/build -o ~/data.osm.pbf -g ~/gps_traces.csv
+          sudo umount /opt/benchmarks
      - name: Run Base Benchmarks
        run: |
-          # we intentionally use scripts from PR branch to be able to update them and see results in the same PR
-          ./pr/scripts/ci/run_benchmarks.sh -f $(pwd)/base -r $(pwd)/base_results -s $(pwd)/pr -b $(pwd)/base/build -o ~/data.osm.pbf -g ~/gps_traces.csv
+          sudo mount -t tmpfs -o size=4g none /opt/benchmarks
+          cp -rf base/build /opt/benchmarks/build
+          mkdir -p /opt/benchmarks/test
+          cp -rf base/test/data /opt/benchmarks/test/data
+          cp -rf base/profiles /opt/benchmarks/profiles

+          # TODO: remove it when base branch will have this file at needed location
+          if [ ! -f /opt/benchmarks/test/data/portugal_to_korea.json ]; then
+            cp base/src/benchmarks/portugal_to_korea.json /opt/benchmarks/test/data/portugal_to_korea.json
+          fi
+          # we intentionally use scripts from PR branch to be able to update them and see results in the same PR
+          ./pr/scripts/ci/run_benchmarks.sh -f /opt/benchmarks -r $(pwd)/base_results -s $(pwd)/pr -b /opt/benchmarks/build -o ~/data.osm.pbf -g ~/gps_traces.csv
+          sudo umount /opt/benchmarks
      - name: Post Benchmark Results
        run: |
          python3 pr/scripts/ci/post_benchmark_results.py base_results pr_results
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -25,6 +25,8 @@
      - CHANGED: Use node-api instead of NAN. [#6452](https://github.com/Project-OSRM/osrm-backend/pull/6452)
    - Misc:
      - CHANGED: Re-use priority queue in StaticRTree. [#6952](https://github.com/Project-OSRM/osrm-backend/pull/6952)
+      - 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)
      - CHANGED: Apply micro-optimisation for Route API. [#6948](https://github.com/Project-OSRM/osrm-backend/pull/6948)
--- a/include/engine/base64.hpp
+++ b/include/engine/base64.hpp
@ -47,24 +47,29 @@ namespace engine
 // Encodes a chunk of memory to Base64.
 inline std::string encodeBase64(const unsigned char *first, std::size_t size)
 {
-    std::vector<unsigned char> bytes{first, first + size};
-    BOOST_ASSERT(!bytes.empty());
+    BOOST_ASSERT(size > 0);

-    std::size_t bytes_to_pad{0};
+    std::string encoded;
+    encoded.reserve(((size + 2) / 3) * 4);

-    while (bytes.size() % 3 != 0)
+    auto padding = (3 - size % 3) % 3;
+
+    BOOST_ASSERT(padding == 0 || padding == 1 || padding == 2);
+
+    for (auto itr = detail::Base64FromBinary(first); itr != detail::Base64FromBinary(first + size);
+         ++itr)
    {
-        bytes_to_pad += 1;
-        bytes.push_back(0);
+        encoded.push_back(*itr);
    }

-    BOOST_ASSERT(bytes_to_pad == 0 || bytes_to_pad == 1 || bytes_to_pad == 2);
-    BOOST_ASSERT_MSG(0 == bytes.size() % 3, "base64 input data size is not a multiple of 3");
+    for (size_t index = 0; index < padding; ++index)
+    {
+        encoded.push_back('=');
+    }

-    std::string encoded{detail::Base64FromBinary{bytes.data()},
-                        detail::Base64FromBinary{bytes.data() + (bytes.size() - bytes_to_pad)}};
+    BOOST_ASSERT(encoded.size() == (size + 2) / 3 * 4);

-    return encoded.append(bytes_to_pad, '=');
+    return encoded;
 }

 // C++11 standard 3.9.1/1: Plain char, signed char, and unsigned char are three distinct types
--- 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<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;
-    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, &current_lat, &current_lon, coordinate_to_polyline](
-            const util::Coordinate loc)
-        {
-            const int lat_diff =
-                std::round(static_cast<int>(loc.lat) * coordinate_to_polyline) - current_lat;
-            const int lon_diff =
-                std::round(static_cast<int>(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<int>(it->lat) * coordinate_to_polyline) - current_lat;
+        const int lon_diff =
+            std::round(static_cast<int>(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
--- a/scripts/ci/run_benchmarks.sh
+++ b/scripts/ci/run_benchmarks.sh
@ -64,7 +64,7 @@ function run_benchmarks_for_folder {
    echo "Running alias"
    $BENCHMARKS_FOLDER/alias-bench > "$RESULTS_FOLDER/alias.bench"
    echo "Running json-render-bench"
-    $BENCHMARKS_FOLDER/json-render-bench  "$FOLDER/src/benchmarks/portugal_to_korea.json" > "$RESULTS_FOLDER/json-render.bench"
+    $BENCHMARKS_FOLDER/json-render-bench  "$FOLDER/test/data/portugal_to_korea.json" > "$RESULTS_FOLDER/json-render.bench"
    echo "Running packedvector-bench"
    $BENCHMARKS_FOLDER/packedvector-bench > "$RESULTS_FOLDER/packedvector.bench"
    echo "Running rtree-bench"
--- 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<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
--- a/src/benchmarks/portugal_to_korea.json
+++ b/src/benchmarks/portugal_to_korea.json
--- a/unit_tests/engine/base64.cpp
+++ b/unit_tests/engine/base64.cpp
@ -74,4 +74,98 @@ BOOST_AUTO_TEST_CASE(hint_encoding_decoding_roundtrip_bytewise)
                           reinterpret_cast<const unsigned char *>(&decoded)));
 }

+BOOST_AUTO_TEST_CASE(long_string_encoding)
+{
+    using namespace osrm::engine;
+    std::string long_string(1000, 'A'); // String of 1000 'A's
+    std::string encoded = encodeBase64(long_string);
+    BOOST_CHECK_EQUAL(decodeBase64(encoded), long_string);
+}
+
+BOOST_AUTO_TEST_CASE(invalid_base64_decoding)
+{
+    using namespace osrm::engine;
+    BOOST_CHECK_THROW(decodeBase64("Invalid!"), std::exception);
+}
+
+BOOST_AUTO_TEST_CASE(hint_serialization_size)
+{
+    using namespace osrm::engine;
+    using namespace osrm::util;
+
+    const Coordinate coordinate;
+    const PhantomNode phantom;
+    const osrm::test::MockDataFacade<osrm::engine::routing_algorithms::ch::Algorithm> facade{};
+
+    const SegmentHint hint{phantom, facade.GetCheckSum()};
+    const auto base64 = hint.ToBase64();
+
+    BOOST_CHECK_EQUAL(base64.size(), 112);
+}
+
+BOOST_AUTO_TEST_CASE(extended_roundtrip_tests)
+{
+    using namespace osrm::engine;
+
+    std::vector<std::string> test_strings = {
+        "Hello, World!",                    // Simple ASCII string
+        "1234567890",                       // Numeric string
+        "!@#$%^&*()_+",                     // Special characters
+        std::string(1000, 'A'),             // Long repeating string
+        "¡Hola, mundo!",                    // Non-ASCII characters
+        "こんにちは、世界！",               // Unicode characters
+        std::string("\x00\x01\x02\x03", 4), // Binary data
+        "a",                                // Single character
+        "ab",                               // Two characters
+        "abc",                              // Three characters (no padding in Base64)
+        std::string(190, 'x')               // String that doesn't align with Base64 padding
+    };
+
+    for (const auto &test_str : test_strings)
+    {
+        std::string encoded = encodeBase64(test_str);
+        std::string decoded = decodeBase64(encoded);
+        BOOST_CHECK_EQUAL(decoded, test_str);
+
+        // Additional checks
+        BOOST_CHECK(encoded.find_first_not_of(
+                        "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=") ==
+                    std::string::npos);
+        if (test_str.length() % 3 != 0)
+        {
+            BOOST_CHECK(encoded.back() == '=');
+        }
+    }
+}
+
+BOOST_AUTO_TEST_CASE(roundtrip_with_url_safe_chars)
+{
+    using namespace osrm::engine;
+
+    std::string original = "Hello+World/Nothing?Is:Impossible";
+    std::string encoded = encodeBase64(original);
+
+    // Replace '+' with '-' and '/' with '_'
+    std::replace(encoded.begin(), encoded.end(), '+', '-');
+    std::replace(encoded.begin(), encoded.end(), '/', '_');
+
+    std::string decoded = decodeBase64(encoded);
+    BOOST_CHECK_EQUAL(decoded, original);
+}
+
+BOOST_AUTO_TEST_CASE(roundtrip_stress_test)
+{
+    using namespace osrm::engine;
+
+    std::string test_str;
+    for (int i = 0; i < 1000; ++i)
+    {
+        test_str += static_cast<char>(i % 256);
+    }
+
+    std::string encoded = encodeBase64(test_str);
+    std::string decoded = decodeBase64(encoded);
+    BOOST_CHECK_EQUAL(decoded, test_str);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
--- 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<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()