Merge branch 'master' into sf-ankerl

This commit is contained in:
Siarhei Fedartsou 2024-06-08 22:29:49 +02:00
commit 2e3f3e90ef
19 changed files with 193 additions and 157 deletions

View File

@ -13,6 +13,10 @@ Checks: >
-bugprone-forward-declaration-namespace,
-bugprone-sizeof-expression,
-bugprone-throw-keyword-missing,
-bugprone-chained-comparison,
-bugprone-incorrect-enable-if,
-bugprone-switch-missing-default-case,
-bugprone-empty-catch,
-clang-analyzer-*,
-clang-diagnostic-deprecated-declarations,
-clang-diagnostic-constant-conversion,
@ -49,11 +53,13 @@ Checks: >
-misc-misplaced-const,
-misc-definitions-in-headers,
-misc-unused-parameters,
-misc-include-cleaner,
modernize-concat-nested-namespaces,
modernize-use-using,
performance-*,
-performance-noexcept-move-constructor,
-performance-no-int-to-ptr,
-performance-enum-size,
-performance-avoid-endl,
readability-*,
-readability-avoid-const-params-in-decls,
-readability-braces-around-statements,
@ -82,6 +88,10 @@ Checks: >
-readability-make-member-function-const,
-readability-redundant-string-init,
-readability-non-const-parameter,
-readability-redundant-inline-specifier,
-readability-avoid-nested-conditional-operator,
-readability-avoid-return-with-void-value,
-readability-redundant-casting,
-readability-static-accessed-through-instance
WarningsAsErrors: '*'

View File

@ -192,14 +192,14 @@ jobs:
CXXCOMPILER: clang++-15
CUCUMBER_TIMEOUT: 60000
- name: clang-15-debug-clang-tidy
- name: clang-18-debug-clang-tidy
continue-on-error: false
node: 18
runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
BUILD_TOOLS: ON
BUILD_TYPE: Debug
CCOMPILER: clang-15
CXXCOMPILER: clang++-15
CCOMPILER: clang-18
CXXCOMPILER: clang++-18
CUCUMBER_TIMEOUT: 60000
ENABLE_CLANG_TIDY: ON
@ -656,7 +656,7 @@ jobs:
path: pr
- name: Install dependencies
run: |
python3 -m pip install "conan<2.0.0" "requests==2.31.0" "locust==2.28.0"
python3 -m pip install "conan<2.0.0" "requests==2.31.0" "numpy==1.26.4"
sudo apt-get update -y && sudo apt-get install ccache
- name: Prepare data
run: |

View File

@ -8,6 +8,7 @@
- ADDED: Add support for opposite approach request parameter. [#6842](https://github.com/Project-OSRM/osrm-backend/pull/6842)
- ADDED: Add support for accessing edge flags in `process_segment` [#6658](https://github.com/Project-OSRM/osrm-backend/pull/6658)
- Build:
- CHANGED: Upgrade clang-format to version 15. [#6919](https://github.com/Project-OSRM/osrm-backend/pull/6919)
- CHANGED: Use Debian Bookworm as base Docker image [#6904](https://github.com/Project-OSRM/osrm-backend/pull/6904)
- CHANGED: Upgrade CI actions to latest versions [#6893](https://github.com/Project-OSRM/osrm-backend/pull/6893)
- CHANGED: Remove outdated warnings #6894 [#6894](https://github.com/Project-OSRM/osrm-backend/pull/6894)
@ -23,6 +24,9 @@
- NodeJS:
- CHANGED: Use node-api instead of NAN. [#6452](https://github.com/Project-OSRM/osrm-backend/pull/6452)
- Misc:
- FIXED: Fix bugprone-unused-return-value clang-tidy warning. [#6934](https://github.com/Project-OSRM/osrm-backend/pull/6934)
- FIXED: Fix performance-noexcept-move-constructor clang-tidy warning. [#6931](https://github.com/Project-OSRM/osrm-backend/pull/6933)
- FIXED: Fix performance-noexcept-swap clang-tidy warning. [#6931](https://github.com/Project-OSRM/osrm-backend/pull/6931)
- CHANGED: Use custom struct instead of std::pair in QueryHeap. [#6921](https://github.com/Project-OSRM/osrm-backend/pull/6921)
- CHANGED: Use std::string_view::starts_with instead of boost::starts_with. [#6918](https://github.com/Project-OSRM/osrm-backend/pull/6918)
- CHANGED: Get rid of boost::math::constants::* and M_PI in favor of std::numbers. [#6916](https://github.com/Project-OSRM/osrm-backend/pull/6916)

View File

@ -215,7 +215,6 @@ inline void read(storage::tar::FileReader &reader,
const std::string &name,
detail::NameTableImpl<Ownership> &name_table)
{
std::string buffer;
util::serialization::read(reader, name, name_table.indexed_data);
}
} // namespace osrm::extractor::serialization

View File

@ -12,7 +12,7 @@ struct header
// explicitly use default copy c'tor as adding move c'tor
header &operator=(const header &other) = default;
header(std::string name, std::string value) : name(std::move(name)), value(std::move(value)) {}
header(header &&other) : name(std::move(other.name)), value(std::move(other.value)) {}
header(header &&other) noexcept : name(std::move(other.name)), value(std::move(other.value)) {}
void clear()
{

View File

@ -61,7 +61,7 @@ class SharedMemory
{
shm = boost::interprocess::xsi_shared_memory(boost::interprocess::open_only, key);
util::Log(logDEBUG) << "opening " << (int)shm.get_shmid() << " from id " << (int)id;
util::Log(logDEBUG) << "opening " << shm.get_shmid() << " from id " << (int)id;
region = boost::interprocess::mapped_region(shm, boost::interprocess::read_only);
}

View File

@ -26,7 +26,7 @@ struct ConcurrentIDMap
mutable UpgradableMutex mutex;
ConcurrentIDMap() = default;
ConcurrentIDMap(ConcurrentIDMap &&other)
ConcurrentIDMap(ConcurrentIDMap &&other) noexcept
{
if (this != &other)
{
@ -36,7 +36,7 @@ struct ConcurrentIDMap
data = std::move(other.data);
}
}
ConcurrentIDMap &operator=(ConcurrentIDMap &&other)
ConcurrentIDMap &operator=(ConcurrentIDMap &&other) noexcept
{
if (this != &other)
{

View File

@ -166,7 +166,7 @@ class DeallocatingVectorIterator
template <typename ElementT> class DeallocatingVector;
template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs);
template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs) noexcept;
template <typename ElementT> class DeallocatingVector
{
@ -204,8 +204,8 @@ template <typename ElementT> class DeallocatingVector
}
// moving is fine
DeallocatingVector(DeallocatingVector &&other) { swap(other); }
DeallocatingVector &operator=(DeallocatingVector &&other)
DeallocatingVector(DeallocatingVector &&other) noexcept { swap(other); }
DeallocatingVector &operator=(DeallocatingVector &&other) noexcept
{
swap(other);
return *this;
@ -221,9 +221,10 @@ template <typename ElementT> class DeallocatingVector
~DeallocatingVector() { clear(); }
friend void swap<>(DeallocatingVector<ElementT> &lhs, DeallocatingVector<ElementT> &rhs);
friend void swap<>(DeallocatingVector<ElementT> &lhs,
DeallocatingVector<ElementT> &rhs) noexcept;
void swap(DeallocatingVector<ElementT> &other)
void swap(DeallocatingVector<ElementT> &other) noexcept
{
std::swap(current_size, other.current_size);
bucket_list.swap(other.bucket_list);
@ -342,7 +343,7 @@ template <typename ElementT> class DeallocatingVector
}
};
template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs)
template <typename T> void swap(DeallocatingVector<T> &lhs, DeallocatingVector<T> &rhs) noexcept
{
lhs.swap(rhs);
}

View File

@ -154,7 +154,7 @@ template <typename EdgeDataT> class DynamicGraph
return *this;
}
DynamicGraph(DynamicGraph &&other)
DynamicGraph(DynamicGraph &&other) noexcept
{
number_of_nodes = other.number_of_nodes;
// atomics can't be moved this is why we need an own constructor
@ -164,7 +164,7 @@ template <typename EdgeDataT> class DynamicGraph
edge_list = std::move(other.edge_list);
}
DynamicGraph &operator=(DynamicGraph &&other)
DynamicGraph &operator=(DynamicGraph &&other) noexcept
{
number_of_nodes = other.number_of_nodes;
// atomics can't be moved this is why we need an own constructor

View File

@ -10,9 +10,9 @@ namespace osrm::util
namespace permutation_detail
{
template <typename T> static inline void swap(T &a, T &b) { std::swap(a, b); }
template <typename T> static inline void swap(T &a, T &b) noexcept { std::swap(a, b); }
static inline void swap(std::vector<bool>::reference a, std::vector<bool>::reference b)
static inline void swap(std::vector<bool>::reference a, std::vector<bool>::reference b) noexcept
{
std::vector<bool>::swap(a, b);
}

102
scripts/ci/e2e_benchmark.py Normal file
View File

@ -0,0 +1,102 @@
import requests
import sys
import random
from collections import defaultdict
import os
import csv
import numpy as np
import time
import argparse
class BenchmarkRunner:
def __init__(self):
self.coordinates = []
self.tracks = defaultdict(list)
gps_traces_file_path = os.path.expanduser('~/gps_traces.csv')
with open(gps_traces_file_path, 'r') as file:
reader = csv.DictReader(file)
for row in reader:
coord = (float(row['Latitude']), float(row['Longitude']))
self.coordinates.append(coord)
self.tracks[row['TrackID']].append(coord)
self.track_ids = list(self.tracks.keys())
def run(self, benchmark_name, host, num_requests, warmup_requests=50):
for _ in range(warmup_requests):
url = self.make_url(host, benchmark_name)
_ = requests.get(url)
times = []
for _ in range(num_requests):
url = self.make_url(host, benchmark_name)
start_time = time.time()
response = requests.get(url)
end_time = time.time()
if response.status_code != 200:
if benchmark_name == 'match':
code = response.json()['code']
if code == 'NoSegment' or code == 'NoMatch':
continue
raise Exception(f"Error: {response.status_code} {response.text}")
times.append((end_time - start_time) * 1000) # convert to ms
return times
def make_url(self, host, benchmark_name):
if benchmark_name == 'route':
start = random.choice(self.coordinates)
end = random.choice(self.coordinates)
start_coord = f"{start[1]:.6f},{start[0]:.6f}"
end_coord = f"{end[1]:.6f},{end[0]:.6f}"
return f"{host}/route/v1/driving/{start_coord};{end_coord}?overview=full&steps=true"
elif benchmark_name == 'table':
num_coords = random.randint(3, 100)
selected_coords = random.sample(self.coordinates, num_coords)
coords_str = ";".join([f"{coord[1]:.6f},{coord[0]:.6f}" for coord in selected_coords])
return f"{host}/table/v1/driving/{coords_str}"
elif benchmark_name == 'match':
num_coords = random.randint(50, 100)
track_id = random.choice(self.track_ids)
track_coords = self.tracks[track_id][:num_coords]
coords_str = ";".join([f"{coord[1]:.6f},{coord[0]:.6f}" for coord in track_coords])
radiues_str = ";".join([f"{random.randint(5, 20)}" for _ in range(len(track_coords))])
return f"{host}/match/v1/driving/{coords_str}?steps=true&radiuses={radiues_str}"
elif benchmark_name == 'nearest':
coord = random.choice(self.coordinates)
coord_str = f"{coord[1]:.6f},{coord[0]:.6f}"
return f"{host}/nearest/v1/driving/{coord_str}"
elif benchmark_name == 'trip':
num_coords = random.randint(2, 10)
selected_coords = random.sample(self.coordinates, num_coords)
coords_str = ";".join([f"{coord[1]:.6f},{coord[0]:.6f}" for coord in selected_coords])
return f"{host}/trip/v1/driving/{coords_str}?steps=true"
else:
raise Exception(f"Unknown benchmark: {benchmark_name}")
def main():
parser = argparse.ArgumentParser(description='Run GPS benchmark tests.')
parser.add_argument('--host', type=str, required=True, help='Host URL')
parser.add_argument('--method', type=str, required=True, choices=['route', 'table', 'match', 'nearest', 'trip'], help='Benchmark method')
parser.add_argument('--num_requests', type=int, required=True, help='Number of requests to perform')
args = parser.parse_args()
random.seed(42)
runner = BenchmarkRunner()
times = runner.run(args.method, args.host, args.num_requests)
print(f'Total: {np.sum(times)}ms')
print(f"Min time: {np.min(times)}ms")
print(f"Mean time: {np.mean(times)}ms")
print(f"Median time: {np.median(times)}ms")
print(f"95th percentile: {np.percentile(times, 95)}ms")
print(f"99th percentile: {np.percentile(times, 99)}ms")
print(f"Max time: {np.max(times)}ms")
if __name__ == '__main__':
main()

View File

@ -1,74 +0,0 @@
from locust import HttpUser, TaskSet, task, between
import csv
import random
from collections import defaultdict
import os
class OSRMTasks(TaskSet):
def on_start(self):
random.seed(42)
self.coordinates = []
self.tracks = defaultdict(list)
gps_traces_file_path = os.path.expanduser('~/gps_traces.csv')
with open(gps_traces_file_path, 'r') as file:
reader = csv.DictReader(file)
for row in reader:
coord = (float(row['Latitude']), float(row['Longitude']))
self.coordinates.append(coord)
self.tracks[row['TrackID']].append(coord)
self.track_ids = list(self.tracks.keys())
@task
def get_route(self):
start = random.choice(self.coordinates)
end = random.choice(self.coordinates)
start_coord = f"{start[1]:.6f},{start[0]:.6f}"
end_coord = f"{end[1]:.6f},{end[0]:.6f}"
self.client.get(f"/route/v1/driving/{start_coord};{end_coord}?overview=full&steps=true", name="route")
@task
def get_table(self):
num_coords = random.randint(3, 100)
selected_coords = random.sample(self.coordinates, num_coords)
coords_str = ";".join([f"{coord[1]:.6f},{coord[0]:.6f}" for coord in selected_coords])
self.client.get(f"/table/v1/driving/{coords_str}", name="table")
@task
def get_match(self):
num_coords = random.randint(50, 100)
track_id = random.choice(self.track_ids)
track_coords = self.tracks[track_id][:num_coords]
coords_str = ";".join([f"{coord[1]:.6f},{coord[0]:.6f}" for coord in track_coords])
radiues_str = ";".join([f"{random.randint(5, 20)}" for _ in range(len(track_coords))])
with self.client.get(f"/match/v1/driving/{coords_str}?steps=true&radiuses={radiues_str}", name="match", catch_response=True) as response:
if response.status_code == 400:
j = response.json()
# it is expected that some of requests will fail with such error: map matching fails sometimes
if j['code'] == 'NoSegment' or j['code'] == 'NoMatch':
response.success()
@task
def get_nearest(self):
coord = random.choice(self.coordinates)
coord_str = f"{coord[1]:.6f},{coord[0]:.6f}"
self.client.get(f"/nearest/v1/driving/{coord_str}", name="nearest")
@task
def get_trip(self):
num_coords = random.randint(2, 10)
selected_coords = random.sample(self.coordinates, num_coords)
coords_str = ";".join([f"{coord[1]:.6f},{coord[0]:.6f}" for coord in selected_coords])
self.client.get(f"/trip/v1/driving/{coords_str}?steps=true", name="trip")
class OSRMUser(HttpUser):
tasks = [OSRMTasks]
# random wait time between requests to not load server for 100%
wait_time = between(0.05, 0.5)

View File

@ -1,31 +0,0 @@
import sys
import csv
def main(locust_csv_base_name, suffix, output_folder):
with open(f"{locust_csv_base_name}_stats.csv", 'r') as file:
reader = csv.DictReader(file)
for row in reader:
name = row['Name']
if name == 'Aggregated': continue
statistics = f'''
requests: {row['Request Count']}
failures: {row['Failure Count']}
req/s: {float(row['Requests/s']):.3f}req/s
avg: {float(row['Average Response Time']):.3f}ms
50%: {row['50%']}ms
75%: {row['75%']}ms
95%: {row['95%']}ms
98%: {row['98%']}ms
99%: {row['99%']}ms
min: {float(row['Min Response Time']):.3f}ms
max: {float(row['Max Response Time']):.3f}ms
'''
with open(f"{output_folder}/e2e_{name}_{suffix}.bench", 'w') as f:
f.write(statistics)
if __name__ == '__main__':
if len(sys.argv) != 4:
print(f"Usage: {sys.argv[0]} <locust csv base name> <suffix> <output folder>")
sys.exit(1)
main(sys.argv[1], sys.argv[2], sys.argv[3])

View File

@ -1,11 +1,24 @@
#!/bin/bash
set -eou pipefail
function measure_peak_ram_and_time {
COMMAND=$1
OUTPUT_FILE=$2
OUTPUT=$(/usr/bin/time -f "%e %M" $COMMAND 2>&1 | tail -n 1)
TIME=$(echo $OUTPUT | awk '{print $1}')
PEAK_RAM_KB=$(echo $OUTPUT | awk '{print $2}')
PEAK_RAM_MB=$(echo "scale=2; $PEAK_RAM_KB / 1024" | bc)
echo "Time: ${TIME}s Peak RAM: ${PEAK_RAM_MB}MB" > $OUTPUT_FILE
}
function run_benchmarks_for_folder {
echo "Running benchmarks for $1"
FOLDER=$1
RESULTS_FOLDER=$2
SCRIPTS_FOLDER=$3
mkdir -p $RESULTS_FOLDER
@ -23,37 +36,33 @@ function run_benchmarks_for_folder {
BINARIES_FOLDER="$FOLDER/build"
cp ~/data.osm.pbf $FOLDER
$BINARIES_FOLDER/osrm-extract -p $FOLDER/profiles/car.lua $FOLDER/data.osm.pbf
$BINARIES_FOLDER/osrm-partition $FOLDER/data.osrm
$BINARIES_FOLDER/osrm-customize $FOLDER/data.osrm
$BINARIES_FOLDER/osrm-contract $FOLDER/data.osrm
if [ -f "$FOLDER/scripts/ci/locustfile.py" ]; then
for ALGORITHM in mld ch; do
$BINARIES_FOLDER/osrm-routed --algorithm $ALGORITHM $FOLDER/data.osrm &
OSRM_ROUTED_PID=$!
measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-extract -p $FOLDER/profiles/car.lua $FOLDER/data.osm.pbf" "$RESULTS_FOLDER/osrm_extract.bench"
measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-partition $FOLDER/data.osrm" "$RESULTS_FOLDER/osrm_partition.bench"
measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-customize $FOLDER/data.osrm" "$RESULTS_FOLDER/osrm_customize.bench"
measure_peak_ram_and_time "$BINARIES_FOLDER/osrm-contract $FOLDER/data.osrm" "$RESULTS_FOLDER/osrm_contract.bench"
# wait for osrm-routed to start
curl --retry-delay 3 --retry 10 --retry-all-errors "http://127.0.0.1:5000/route/v1/driving/13.388860,52.517037;13.385983,52.496891?steps=true"
locust -f $FOLDER/scripts/ci/locustfile.py \
--headless \
--processes -1 \
--users 10 \
--spawn-rate 1 \
--host http://localhost:5000 \
--run-time 1m \
--csv=locust_results_$ALGORITHM \
--loglevel ERROR
for ALGORITHM in ch mld; do
$BINARIES_FOLDER/osrm-routed --algorithm $ALGORITHM $FOLDER/data.osrm &
OSRM_ROUTED_PID=$!
python3 $FOLDER/scripts/ci/process_locust_benchmark_results.py locust_results_$ALGORITHM $ALGORITHM $RESULTS_FOLDER
# wait for osrm-routed to start
if ! curl --retry-delay 3 --retry 10 --retry-all-errors "http://127.0.0.1:5000/route/v1/driving/13.388860,52.517037;13.385983,52.496891?steps=true"; then
echo "osrm-routed failed to start for algorithm $ALGORITHM"
kill -9 $OSRM_ROUTED_PID
continue
fi
kill -0 $OSRM_ROUTED_PID
for METHOD in route nearest trip table match; do
python3 $SCRIPTS_FOLDER/scripts/ci/e2e_benchmark.py --host http://localhost:5000 --method $METHOD --num_requests 1000 > $RESULTS_FOLDER/e2e_${METHOD}_${ALGORITHM}.bench
done
fi
kill -9 $OSRM_ROUTED_PID
done
}
run_benchmarks_for_folder $1 "${1}_results"
run_benchmarks_for_folder $2 "${2}_results"
run_benchmarks_for_folder $1 "${1}_results" $2
run_benchmarks_for_folder $2 "${2}_results" $2

View File

@ -186,6 +186,7 @@ void Connection::handle_timeout(boost::system::error_code ec)
if (ec != boost::asio::error::operation_aborted)
{
boost::system::error_code ignore_error;
// NOLINTNEXTLINE(bugprone-unused-return-value)
TCP_socket.cancel(ignore_error);
handle_shutdown();
}
@ -197,6 +198,7 @@ void Connection::handle_shutdown()
timer.cancel();
// Initiate graceful connection closure.
boost::system::error_code ignore_error;
// NOLINTNEXTLINE(bugprone-unused-return-value)
TCP_socket.shutdown(boost::asio::ip::tcp::socket::shutdown_both, ignore_error);
}

View File

@ -19416,7 +19416,14 @@ namespace sol { namespace function_detail {
}
template <bool is_yielding, bool no_trampoline>
static int call(lua_State* L) noexcept(std::is_nothrow_copy_assignable_v<T>) {
static int call(lua_State* L)
// see https://github.com/ThePhD/sol2/issues/1581#issuecomment-2103463524
#if SOL_IS_ON(SOL_COMPILER_CLANG)
// apparent regression in clang 18 - llvm/llvm-project#91362
#else
noexcept(std::is_nothrow_copy_assignable_v<T>)
#endif
{
int nr;
if constexpr (no_trampoline) {
nr = real_call(L);
@ -19456,7 +19463,14 @@ namespace sol { namespace function_detail {
}
template <bool is_yielding, bool no_trampoline>
static int call(lua_State* L) noexcept(std::is_nothrow_copy_assignable_v<T>) {
static int call(lua_State* L)
// see https://github.com/ThePhD/sol2/issues/1581#issuecomment-2103463524
#if SOL_IS_ON(SOL_COMPILER_CLANG)
// apparent regression in clang 18 - llvm/llvm-project#91362
#else
noexcept(std::is_nothrow_copy_assignable_v<T>)
#endif
{
int nr;
if constexpr (no_trampoline) {
nr = real_call(L);

View File

@ -239,7 +239,6 @@ void test_route_same_coordinates(bool use_json_only_api)
BOOST_CHECK(((void)name, true));
// nothing can be said about mode, contains mode of transportation
const auto mode = std::get<json::String>(step_object.values.at("mode")).value;
BOOST_CHECK(!name.empty());
const auto &maneuver =

View File

@ -16,12 +16,15 @@ using namespace osrm::util;
BOOST_AUTO_TEST_SUITE(graph_view)
static void shuffle(std::vector<EdgeWithSomeAdditionalData> &grid_edges)
namespace
{
void shuffle(std::vector<EdgeWithSomeAdditionalData> &grid_edges)
{
std::random_device rd;
std::mt19937 rng(rd());
std::shuffle(grid_edges.begin(), grid_edges.end(), rng);
}
} // namespace
BOOST_AUTO_TEST_CASE(separate_top_bottom)
{

View File

@ -235,8 +235,6 @@ auto make_rtree(const boost::filesystem::path &path, FixtureT &fixture)
template <typename RTreeT = TestStaticRTree, typename FixtureT>
void construction_test(const std::string &path, FixtureT &fixture)
{
std::string leaves_path;
std::string nodes_path;
auto rtree = make_rtree<RTreeT>(path, fixture);
LinearSearchNN<TestData> lsnn(fixture.coords, fixture.edges);