Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to clang-tidy 18 #6919

Merged
merged 29 commits into from
Jun 6, 2024
Merged

Upgrade to clang-tidy 18 #6919

merged 29 commits into from
Jun 6, 2024

Conversation

SiarheiFedartsou
Copy link
Member

@SiarheiFedartsou SiarheiFedartsou commented May 30, 2024

Benchmark Results

Benchmark Base PR
alias aliased u32: 1223.27
plain u32: 1213.06
aliased double: 1279.07
plain double: 1272.1
aliased u32: 1180.83
plain u32: 1170.75
aliased double: 1236.3
plain double: 1224.54
e2e_match_ch requests: 381
failures: 0
req/s: 6.352req/s
avg: 5.031ms
50%: 3ms
75%: 4ms
95%: 13ms
98%: 30ms
99%: 45ms
min: 1.318ms
max: 78.540ms
requests: 362
failures: 0
req/s: 6.038req/s
avg: 5.257ms
50%: 3ms
75%: 4ms
95%: 11ms
98%: 33ms
99%: 63ms
min: 1.262ms
max: 76.613ms
e2e_match_mld requests: 342
failures: 0
req/s: 5.705req/s
avg: 5.362ms
50%: 4ms
75%: 4ms
95%: 15ms
98%: 30ms
99%: 42ms
min: 1.353ms
max: 60.721ms
requests: 379
failures: 0
req/s: 6.317req/s
avg: 5.328ms
50%: 3ms
75%: 4ms
95%: 15ms
98%: 28ms
99%: 59ms
min: 1.362ms
max: 97.454ms
e2e_nearest_ch requests: 438
failures: 0
req/s: 7.303req/s
avg: 1.697ms
50%: 2ms
75%: 2ms
95%: 2ms
98%: 2ms
99%: 2ms
min: 1.169ms
max: 4.689ms
requests: 411
failures: 0
req/s: 6.855req/s
avg: 1.624ms
50%: 2ms
75%: 2ms
95%: 2ms
98%: 3ms
99%: 3ms
min: 1.066ms
max: 6.116ms
e2e_nearest_mld requests: 435
failures: 0
req/s: 7.257req/s
avg: 1.897ms
50%: 2ms
75%: 2ms
95%: 2ms
98%: 3ms
99%: 3ms
min: 1.219ms
max: 52.939ms
requests: 408
failures: 0
req/s: 6.801req/s
avg: 1.605ms
50%: 2ms
75%: 2ms
95%: 2ms
98%: 2ms
99%: 2ms
min: 1.032ms
max: 6.054ms
e2e_route_ch requests: 382
failures: 0
req/s: 6.369req/s
avg: 4.755ms
50%: 4ms
75%: 5ms
95%: 7ms
98%: 9ms
99%: 10ms
min: 1.467ms
max: 101.833ms
requests: 378
failures: 0
req/s: 6.304req/s
avg: 4.746ms
50%: 4ms
75%: 5ms
95%: 6ms
98%: 8ms
99%: 10ms
min: 1.476ms
max: 143.283ms
e2e_route_mld requests: 384
failures: 0
req/s: 6.406req/s
avg: 5.409ms
50%: 5ms
75%: 6ms
95%: 7ms
98%: 10ms
99%: 11ms
min: 1.386ms
max: 99.600ms
requests: 412
failures: 0
req/s: 6.868req/s
avg: 4.547ms
50%: 4ms
75%: 5ms
95%: 6ms
98%: 8ms
99%: 10ms
min: 1.610ms
max: 98.198ms
e2e_table_ch requests: 376
failures: 0
req/s: 6.269req/s
avg: 49.679ms
50%: 30ms
75%: 87ms
95%: 130ms
98%: 140ms
99%: 140ms
min: 3.030ms
max: 274.284ms
requests: 392
failures: 0
req/s: 6.538req/s
avg: 56.104ms
50%: 38ms
75%: 94ms
95%: 130ms
98%: 140ms
99%: 150ms
min: 2.608ms
max: 332.286ms
e2e_table_mld requests: 371
failures: 0
req/s: 6.189req/s
avg: 75.046ms
50%: 77ms
75%: 110ms
95%: 140ms
98%: 150ms
99%: 150ms
min: 4.686ms
max: 183.313ms
requests: 414
failures: 0
req/s: 6.901req/s
avg: 45.394ms
50%: 28ms
75%: 75ms
95%: 130ms
98%: 140ms
99%: 150ms
min: 2.660ms
max: 159.087ms
e2e_trip_ch requests: 363
failures: 0
req/s: 6.052req/s
avg: 18.241ms
50%: 17ms
75%: 24ms
95%: 32ms
98%: 34ms
99%: 36ms
min: 2.449ms
max: 41.356ms
requests: 341
failures: 0
req/s: 5.687req/s
avg: 18.049ms
50%: 17ms
75%: 24ms
95%: 32ms
98%: 33ms
99%: 34ms
min: 2.060ms
max: 37.663ms
e2e_trip_mld requests: 354
failures: 0
req/s: 5.906req/s
avg: 21.897ms
50%: 21ms
75%: 28ms
95%: 35ms
98%: 42ms
99%: 44ms
min: 4.209ms
max: 46.124ms
requests: 354
failures: 0
req/s: 5.901req/s
avg: 16.214ms
50%: 15ms
75%: 21ms
95%: 31ms
98%: 33ms
99%: 35ms
min: 2.675ms
max: 44.021ms
json-render String: 6.75339ms
Stringstream: 9.4243ms
Vector: 6.98778ms
String: 6.70346ms
Stringstream: 9.32243ms
Vector: 6.91688ms
match_ch Default radius:
4.41954ms/req at 82 coordinate
0.0538968ms/coordinate
Radius 5m:
4.38232ms/req at 82 coordinate
0.0534429ms/coordinate
Radius 10m:
14.9263ms/req at 82 coordinate
0.182028ms/coordinate
Radius 15m:
36.6036ms/req at 82 coordinate
0.446385ms/coordinate
Radius 30m:
312.995ms/req at 82 coordinate
3.81701ms/coordinate
Default radius:
4.45352ms/req at 82 coordinate
0.0543112ms/coordinate
Radius 5m:
4.43942ms/req at 82 coordinate
0.0541393ms/coordinate
Radius 10m:
15.1616ms/req at 82 coordinate
0.184898ms/coordinate
Radius 15m:
37.1285ms/req at 82 coordinate
0.452786ms/coordinate
Radius 30m:
315.59ms/req at 82 coordinate
3.84866ms/coordinate
match_mld Default radius:
2.76855ms/req at 82 coordinate
0.0337628ms/coordinate
Radius 5m:
2.83892ms/req at 82 coordinate
0.0346209ms/coordinate
Radius 10m:
10.1743ms/req at 82 coordinate
0.124077ms/coordinate
Radius 15m:
26.1871ms/req at 82 coordinate
0.319354ms/coordinate
Radius 30m:
302.946ms/req at 82 coordinate
3.69446ms/coordinate
Default radius:
2.75393ms/req at 82 coordinate
0.0335845ms/coordinate
Radius 5m:
2.73113ms/req at 82 coordinate
0.0333065ms/coordinate
Radius 10m:
10.0511ms/req at 82 coordinate
0.122574ms/coordinate
Radius 15m:
25.7732ms/req at 82 coordinate
0.314308ms/coordinate
Radius 30m:
302.781ms/req at 82 coordinate
3.69245ms/coordinate
packedvector random write:
std::vector 12147.1 ms
util::packed_vector 81904.8 ms
slowdown: 6.74273
random read:
std::vector 12018.3 ms
util::packed_vector 33614.6 ms
slowdown: 2.79696
random write:
std::vector 11579.4 ms
util::packed_vector 73798.2 ms
slowdown: 6.37324
random read:
std::vector 11333.3 ms
util::packed_vector 30349.3 ms
slowdown: 2.6779
route_ch 1000 routes, 3 coordinates, no alternatives, overview=full, steps=true
514.012ms
0.514012ms/req
1000 routes, 2 coordinates, no alternatives, overview=full, steps=true
351.655ms
0.351655ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=full, steps=true
629.65ms
0.62965ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false
151.993ms
0.151993ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false
97.6789ms
0.0976789ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false
133.34ms
0.13334ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false, radius=750
152.201ms
0.152201ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false, radius=750
98.2297ms
0.0982297ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false, radius=750
132.641ms
0.132641ms/req
1000 routes, 3 coordinates, no alternatives, overview=full, steps=true
511.213ms
0.511213ms/req
1000 routes, 2 coordinates, no alternatives, overview=full, steps=true
353.241ms
0.353241ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=full, steps=true
667.345ms
0.667345ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false
150.854ms
0.150854ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false
97.4697ms
0.0974697ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false
133.423ms
0.133423ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false, radius=750
151.391ms
0.151391ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false, radius=750
97.2257ms
0.0972257ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false, radius=750
131.898ms
0.131898ms/req
route_mld 1000 routes, 3 coordinates, no alternatives, overview=full, steps=true
639.617ms
0.639617ms/req
1000 routes, 2 coordinates, no alternatives, overview=full, steps=true
439.342ms
0.439342ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=full, steps=true
814.715ms
0.814715ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false
263.551ms
0.263551ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false
160.555ms
0.160555ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false
283.312ms
0.283312ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false, radius=750
259.53ms
0.25953ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false, radius=750
159.99ms
0.15999ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false, radius=750
282.618ms
0.282618ms/req
1000 routes, 3 coordinates, no alternatives, overview=full, steps=true
646.409ms
0.646409ms/req
1000 routes, 2 coordinates, no alternatives, overview=full, steps=true
441.817ms
0.441817ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=full, steps=true
820.048ms
0.820048ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false
266.992ms
0.266992ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false
162.773ms
0.162773ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false
285.496ms
0.285496ms/req
1000 routes, 3 coordinates, no alternatives, overview=false, steps=false, radius=750
258.837ms
0.258837ms/req
1000 routes, 2 coordinates, no alternatives, overview=false, steps=false, radius=750
160.161ms
0.160161ms/req
1000 routes, 2 coordinates, 3 alternatives, overview=false, steps=false, radius=750
286.399ms
0.286399ms/req
rtree 1 result:
206.729ms -> 0.0206729 ms/query
10 results:
242.198ms -> 0.0242198 ms/query
1 result:
207.879ms -> 0.0207879 ms/query
10 results:
242.733ms -> 0.0242733 ms/query

@SiarheiFedartsou SiarheiFedartsou changed the title Use clang-tidy 18 Upgrade to clang-tidy 18 Jun 5, 2024
@@ -13,6 +13,11 @@ Checks: >
-bugprone-forward-declaration-namespace,
-bugprone-sizeof-expression,
-bugprone-throw-keyword-missing,
-bugprone-chained-comparison,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of these warnings seem to be useful - I'll create an issue to fix some of them before merging it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -19416,7 +19416,13 @@ 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)
#if SOL_IS_ON(SOL_COMPILER_CLANG)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took this workaround from ThePhD/sol2#1581 (comment)

I don't see another way to build with newer clang unfortunately. I'll try to make a PR to have this workaround in sol2 mainline...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a link as a comment right in the source. This way one can trace the origin of this bandaid, and eventually remove it in the future.

@SiarheiFedartsou SiarheiFedartsou marked this pull request as ready for review June 6, 2024 16:33
Copy link
Collaborator

@DennisOSRM DennisOSRM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see the toolchain get updated. 👍🏽

@@ -703,7 +703,7 @@ jobs:
run: |
ccache -p
ccache -s

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit pick: stray whitespace 😉

@@ -19416,7 +19416,13 @@ 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)
#if SOL_IS_ON(SOL_COMPILER_CLANG)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a link as a comment right in the source. This way one can trace the origin of this bandaid, and eventually remove it in the future.

@@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above looks like it can be removed, too.

@SiarheiFedartsou SiarheiFedartsou merged commit 523ee76 into master Jun 6, 2024
20 of 21 checks passed
@SiarheiFedartsou SiarheiFedartsou deleted the sf-clang-tidy-18 branch June 6, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants