opm-common
opm-common copied to clipboard
OPM
GasPvtMultiplexer: Replace unholy trinity with visitor overload sets
Sits on top of https://github.com/OPM/opm-common/pull/3278 Waiting for https://github.com/OPM/opm-common/pull/3246
I realize this may be controversial, and that it might have runtime implications (which we need to benchmark) but; This replaces what I consider the unholy trinity (void pointers, macros and SFINAE) with the visitor overload set idiom in the gas multiplexer.
As a bonus we avoid the need to have explicit ctors, copy ctors, assignment and comparison operators.
As a bonus we avoid the need to have explicit ctors, copy ctors, assignment and comparison operators.
:tada:
Benchmark result overview:
| Test | Configuration | Relative |
|---|---|---|
| opm-git | OPM Benchmark: drogon - Threads: 1 | 1.026 |
| opm-git | OPM Benchmark: drogon - Threads: 8 | 0.989 |
| opm-git | OPM Benchmark: smeaheia - Threads: 1 | 1.02 |
| opm-git | OPM Benchmark: smeaheia - Threads: 8 | 0.906 |
| opm-git | OPM Benchmark: flow_mpi_extra - Threads: 1 | 1.001 |
| opm-git | OPM Benchmark: flow_mpi_extra - Threads: 8 | 0.901 |
| opm-git | OPM Benchmark: flow_mpi_norne - Threads: 1 | 0.995 |
| opm-git | OPM Benchmark: flow_mpi_norne - Threads: 8 | 0.983 |
- Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *
View result details @ https://www.ytelses.com/opm/?page=result&id=1899
Benchmark result overview:
| Test | Configuration | Relative |
|---|---|---|
| opm-git | OPM Benchmark: drogon - Threads: 1 | 0.989 |
| opm-git | OPM Benchmark: drogon - Threads: 8 | 0.982 |
| opm-git | OPM Benchmark: smeaheia - Threads: 1 | 1.006 |
| opm-git | OPM Benchmark: smeaheia - Threads: 8 | 0.992 |
| opm-git | OPM Benchmark: flow_mpi_extra - Threads: 1 | 0.987 |
| opm-git | OPM Benchmark: flow_mpi_extra - Threads: 8 | 0.983 |
| opm-git | OPM Benchmark: flow_mpi_norne - Threads: 1 | 0.989 |
| opm-git | OPM Benchmark: flow_mpi_norne - Threads: 8 | 0.976 |
- Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *
View result details @ https://www.ytelses.com/opm/?page=result&id=1912
I did my own benchmarking, and sadly it seems the variant approach is significantly slower (this is with the whole PR chain);
Total time (10 samples) for NORNE_ATW2013
| procs | orig | variant |
|---|---|---|
| 1 | 520.49 +- 3.53 | 551.50 +- 1.57 (p=0.00) |
| 8 | 90.77 +- 0.34 | 99.45 +- 0.44 (p=0.00) |
sadly it seems the variant approach is significantly slower
That's disappointing.