-
Notifications
You must be signed in to change notification settings - Fork 582
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
drt: Use boost flat hashmap when beneficial #6253
base: master
Are you sure you want to change the base?
drt: Use boost flat hashmap when beneficial #6253
Conversation
Please fix the missing DCO (see the details like). |
Please also merge or rebase the master branch into your branch to get the latest changes to CI so pr-head can build. |
Signed-off-by: Szymon Gizler <[email protected]>
00e7666
to
b9f8bef
Compare
Signed-off-by: Szymon Gizler <[email protected]>
Signed-off-by: Szymon Gizler <[email protected]>
Signed-off-by: Szymon Gizler <[email protected]>
825cef5
to
6798309
Compare
Signed-off-by: Szymon Gizler <[email protected]>
e536182
to
e33d5b2
Compare
It seems, that after rebasing on @93819d5, `node_map_` map started being order-sensitive what prevents us from converting it to hashmap Signed-off-by: Szymon Gizler <[email protected]>
e33d5b2
to
6cd3fbe
Compare
Rebasing introduced regression (one map I converted before, is now used in way that seems to depend on element ordering of
|
Branch | Min [s] | Max [s] | Mean [s] | Relative mean | Median [s] | Relative median |
---|---|---|---|---|---|---|
boost-flat-hashmap-opt |
325.45 | 327.16 | 326.17 ± 0.50 | 1.00 ± 0.00 | 326.11 | 1.00 |
master |
327.00 | 330.13 | 328.58 ± 0.90 | 1.01 ± 0.00 | 328.55 | 1.01 |
ibex
, PDK: nangate45
Branch | Min [s] | Max [s] | Mean [s] | Relative mean | Median [s] | Relative median |
---|---|---|---|---|---|---|
boost-flat-hashmap-opt |
131.23 | 132.19 | 131.61 ± 0.34 | 1.00 ± 0.00 | 131.59 | 1.00 |
master |
132.45 | 133.45 | 132.94 ± 0.31 | 1.01 ± 0.00 | 132.90 | 1.01 |
black_parrot
, PDK: nangate45
Branch | Min [s] | Max [s] | Mean [s] | Relative mean | Median [s] | Relative median |
---|---|---|---|---|---|---|
boost-flat-hashmap-opt |
1316.26 | 1319.54 | 1317.98 ± 1.04 | 1.00 ± 0.00 | 1318.08 | 1.00 |
master |
1327.30 | 1330.67 | 1329.28 ± 0.99 | 1.01 ± 0.00 | 1329.08 | 1.01 |
I think it is ready for review now.
find_package(Boost REQUIRED) | ||
find_package(Boost 1.81.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason to drop REQUIRED?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it order to provide custom error message with suggestion about rerunning dependency installer.
Are the results identical or does this require a full test |
Results were identical in cases I checked |
boost
to get flat hashtablesstd::set
indexed by pair of Points with boostboost::unordered_flat_set
std::map
indexed by Point3D withboost::unordered_flat_map
std::unordered_map
withboost::unordered_flat_map
(this was the single case, in which DRT already used hashtable rather than search tree)Benchmarks of
drt
:ibex
, PDK:asap7
boost-flat-hashmap-opt
ibex
, PDK:nangate45
boost-flat-hashmap-opt
black_parrot
, PDK:nangate45
boost-flat-hashmap-opt