[PATCH v4] test: bench_misc: short and long time stamps #1956
Conversation
odpbuild
changed the title
test/performance/odp_bench_misc.c
Outdated
| @@ -103,6 +113,12 @@ static void init_time_global(void) | |||
| for (i = 0; i < REPEAT_COUNT; i++) | |||
| t2[i] = odp_time_global(); | |||
|
|
|||
| for (i = 0; i < REPEAT_COUNT; i++) | |||
| t4[i] = odp_time_global_from_ns(ODP_TIME_MSEC_IN_NS); | |||
There was a problem hiding this comment.
I suggest having a separate init-functions for the time-from-ns tests so that the init function of other tests is not affected. Now the added array init is done for all tests that share the init function, even though they do not use the added t4 and t5 arrays. And since the init function is run after the measurement warmup, any side effects (like cache evictions of the actually used data) can affect the measurements. With separate init functions the actual test function could be exactly the same for the two added tests, which would be useful in making sure that the "short" and "long" tests test exactly the same thing.
The test warmup probably should be improved by having a test specific warmup round after the init phase, but that requires changes in the common bench API. In the mean time having a separate init and not introducing t4 and t5 would minimize the effect of this change to existing tests.
The warmup implementation in the common bench framework is also otherwise problematic: it runs all warmups before any test runs, which is too early. PR 1959 fixes that but leaves the init-after-warmup problem as is.
There was a problem hiding this comment.
Changed in v2 initialization to happen only once in main thread startup.
psavol
force-pushed
the
bench_misc-short-long-time-stamp
branch
from
b8abe2a
to
26dd89d
Compare
odpbuild
changed the title
test/performance/odp_bench_misc.c
Outdated
| @@ -698,7 +719,8 @@ bench_info_t test_suite[] = { | |||
| BENCH_INFO(time_diff, init_time_local, 0, "time_diff (local)"), | |||
| BENCH_INFO(time_diff_ns, init_time_global, 0, NULL), | |||
| BENCH_INFO(time_sum, init_time_global, 0, NULL), | |||
| BENCH_INFO(time_to_ns, init_time_global, 0, NULL), | |||
| BENCH_INFO(time_to_ns_short, init_time_global, 0, "time_to_ns (short)"), | |||
| BENCH_INFO(time_to_ns_long, init_time_global, 0, "time_to_ns (long)"), | |||
There was a problem hiding this comment.
Since init function is not needed anymore in v2, the init function pointer should be set to NULL. With that change this looks ok, as long as PR 1959 goes in first (because init is no longer touching the source data, the improved warmup is needed to bring it to cache before the actual test run).
There was a problem hiding this comment.
Init set to NULL in v3
psavol
force-pushed
the
bench_misc-short-long-time-stamp
branch
from
26dd89d
to
c8e85a9
Compare
odpbuild
changed the title
Time stamp value may affect result of some test cases. Also, test results are more consistent with fixed time stamp values than currrent time (changes between test runs). Added short (1 msec) and long (10 sec) time stamps, and use those in short/long time_to_nsec test cases. Signed-off-by: Petri Savolainen <petri.savolainen@nokia.com> Reviewed-by: Janne Peltonen <janne.peltonen@nokia.com>
psavol
force-pushed
the
bench_misc-short-long-time-stamp
branch
from
c8e85a9
to
3bf1d5f
Compare
odpbuild
changed the title
Time stamp value may affect result of some test cases. Also, test results are more consistent with fixed time stamp values than currrent time (changes between test runs). Added short (1 msec) and long (10 sec) time stamps, and use those in short/long time_to_nsec test cases.