fzsync: limit sampling time

Fixes #429

Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far.

Signed-off-by Jan Stancek <[email protected]>

---
 include/tst_fuzzy_sync.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

include/tst_fuzzy_sync.h +2 -0

index 03f69b78bc82..0ca292c141c3 100644
@@ -162,6 +162,7 @@ struct tst_fzsync_pair {
         *
         * Defaults to 0.5 (~150 seconds with default timeout).
         */
+       float max_sampling_p;
        float exec_time_p;
        /** Internal; The test time remaining on tst_fzsync_pair_reset() */
        float exec_time_start;

@@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
        CHK(avg_alpha, 0, 1, 0.25);
        CHK(min_samples, 20, INT_MAX, 1024);
        CHK(max_dev_ratio, 0, 1, 0.1);
+       CHK(max_sampling_p, 0, 1, 0.25);
        CHK(exec_time_p, 0, 1, 0.5);
        CHK(exec_loops, 20, INT_MAX, 3000000);
 }

Quoted 8 lines

Fixes #429

Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far.

This patch makes sense.

Quoted 35 lines

Signed-off-by Jan Stancek <[email protected]>

---
 include/tst_fuzzy_sync.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

include/tst_fuzzy_sync.h +2 -0

index 03f69b78bc82..0ca292c141c3 100644
@@ -162,6 +162,7 @@ struct tst_fzsync_pair {
         *
         * Defaults to 0.5 (~150 seconds with default timeout).
         */
+       float max_sampling_p;
        float exec_time_p;
        /** Internal; The test time remaining on tst_fzsync_pair_reset() */
        float exec_time_start;

@@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair
 *pair)
        CHK(avg_alpha, 0, 1, 0.25);
        CHK(min_samples, 20, INT_MAX, 1024);
        CHK(max_dev_ratio, 0, 1, 0.1);
+       CHK(max_sampling_p, 0, 1, 0.25);
        CHK(exec_time_p, 0, 1, 0.5);
        CHK(exec_loops, 20, INT_MAX, 3000000);

}

include/tst_fuzzy_sync.h +3 -0

@@ -582,9 +584,18 @@ static inline void tst_fzsync_wait_b(struct
 tst_fzsync_pair *pair)
 static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair)
 {
        int exit = 0;
+       float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start;
+
+       /* Limit amount of time spent on sampling */

include/tst_fuzzy_sync.h static inline void tst_fzsync_wait_b(struct · line 594

Quoted 4 lines

include/tst_fuzzy_sync.h +4 -0

+       if ((pair->max_sampling_p < rem_p)
+               && (pair->sampling > 0)) {
+               tst_res(TINFO, "stopping sampling at %d samples",
+                       pair->sampling);

pair->sampling is the number of samples left, so I think the prints might be more precise if calculate the 'pair->min_samples - pair->sampling'(or pair->exec_loop)?

Quoted 11 lines

include/tst_fuzzy_sync.h +4 -2

+               pair->sampling = 0;
+               tst_fzsync_pair_info(pair);
+       }
 
-       if (pair->exec_time_p
-           < 1 - tst_timeout_remaining() / pair->exec_time_start) {
+       if (pair->exec_time_p < rem_p) {
                tst_res(TINFO,
                        "Exceeded execution time, requesting exit");

                exit = 1;

After involving max_sampling_p, here the warning prints will no longer be useful because it will never get 'pair->sampling > 0' when exec_time_p is exhausted.

Base on your patch I suggest that:

include/tst_fuzzy_sync.h +1 -1

@@ -590,7 +590,7 @@ static inline int tst_fzsync_run_a(struct
 tst_fzsync_pair *pair)
        if ((pair->max_sampling_p < rem_p)
                && (pair->sampling > 0)) {
                tst_res(TINFO, "stopping sampling at %d samples",
-                       pair->sampling);
+                       pair->exec_loop);
                pair->sampling = 0;
                tst_fzsync_pair_info(pair);

        }

include/tst_fuzzy_sync.h +0 -5

@@ -599,11 +599,6 @@ static inline int tst_fzsync_run_a(struct
 tst_fzsync_pair *pair)
                tst_res(TINFO,
                        "Exceeded execution time, requesting exit");
                exit = 1;
-
-               if (pair->sampling > 0) {
-                       tst_res(TWARN,
-                               "Still sampling, consider increasing
 LTP_TIMEOUT_MUL");
-               }
        }

        if (++pair->exec_loop > pair->exec_loops) {

======= Snip ========

Additionally, If we go this way to limit the sampling time, seems 'pair->min_samples' will become less meaning for fuzzy sync library,

because we can just do sampling in a limited time and why we need the minimal samples?

So what about using limited-time to replace min_samples? Maybe like:

include/tst_fuzzy_sync.h +6 -12

@@ -123,19 +123,14 @@ struct tst_fzsync_pair {
         */
        int delay;
        /**
-        *  Internal; The number of samples left or the sampling state.
+        *  Internal; The number of samples in the sampling phase.
         *
-        *  A positive value is the number of remaining mandatory
+        *  A positive value is the number of past mandatory
         *  samples. Zero or a negative indicate some other state.
         */
        int sampling;
-       /**
-        * The Minimum number of statistical samples which must be collected.
-        *
-        * The minimum number of iterations which must be performed before a
-        * random delay can be calculated. Defaults to 1024.
-        */
-       int min_samples;
+       /** Internal; Exit sampling phase if this value is 1 */
+       int sampling_exit;
        /**
         * The maximum allowed proportional average deviation.
         *

@@ -198,7 +193,6 @@ struct tst_fzsync_pair {
 static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
 {
        CHK(avg_alpha, 0, 1, 0.25);
-       CHK(min_samples, 20, INT_MAX, 1024);
        CHK(max_dev_ratio, 0, 1, 0.1);
        CHK(max_sampling_p, 0, 1, 0.25);
        CHK(exec_time_p, 0, 1, 0.5);

@@ -262,7 +256,8 @@ static void tst_fzsync_pair_reset(struct
 tst_fzsync_pair *pair,
        tst_init_stat(&pair->diff_ab);
        tst_init_stat(&pair->spins_avg);
        pair->delay = 0;
-       pair->sampling = pair->min_samples;
+       pair->sampling = 0;
+       pair->sampling_exit = 0;
 
        pair->exec_loop = 0;

@@ -295,7 +290,6 @@ static inline void tst_fzsync_stat_info(struct
 tst_fzsync_stat stat,
  */
 static void tst_fzsync_pair_info(struct tst_fzsync_pair *pair)
 {
-       tst_res(TINFO, "loop = %d", pair->exec_loop);
        tst_fzsync_stat_info(pair->diff_ss, "ns", "start_a - start_b");
        tst_fzsync_stat_info(pair->diff_sa, "ns", "end_a - start_a");

        tst_fzsync_stat_info(pair->diff_sb, "ns", "end_b - start_b");

include/tst_fuzzy_sync.h +3 -0

@@ -450,6 +444,9 @@ static void tst_fzsync_pair_update(struct
 tst_fzsync_pair *pair)
        float alpha = pair->avg_alpha;
        float per_spin_time, time_delay, dev_ratio;
 
+       if (!pair->sampling_exit)
+               ++pair->sampling;
+
        dev_ratio = (pair->diff_sa.dev_ratio
                     + pair->diff_sb.dev_ratio

+ pair->diff_ab.dev_ratio

include/tst_fuzzy_sync.h +5 -3

@@ -465,11 +462,13 @@ static void tst_fzsync_pair_update(struct
 tst_fzsync_pair *pair)
                tst_upd_diff_stat(&pair->diff_ab, alpha,
                                  pair->a_end, pair->b_end);
                tst_upd_stat(&pair->spins_avg, alpha, pair->spins);
-               if (pair->sampling > 0 && --pair->sampling == 0) {
+               if (pair->sampling_exit && pair->sampling > 0) {
                        tst_res(TINFO,
-                               "Minimum sampling period ended,
 deviation ratio = %.2f",
-                               dev_ratio);
+                               "Sampling period ended, total samples = %d,"
+                               "deviation ratio = %.2f",
+                               pair->sampling, dev_ratio);
                        tst_fzsync_pair_info(pair);
+                       pair->sampling = 0;
                }

        } else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1)
        {
                per_spin_time = fabsf(pair->diff_ab.avg) /
                pair->spins_avg.avg;

include/tst_fuzzy_sync.h +3 -12

@@ -587,23 +586,14 @@ static inline int tst_fzsync_run_a(struct
 tst_fzsync_pair *pair)
        float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start;
 
        /* Limit amount of time spent on sampling */
-       if ((pair->max_sampling_p < rem_p)
-               && (pair->sampling > 0)) {
-               tst_res(TINFO, "stopping sampling at %d samples",
-                       pair->sampling);
-               pair->sampling = 0;
-               tst_fzsync_pair_info(pair);
-       }
+       if ((pair->max_sampling_p < rem_p && pair->sampling > 0)
+                       || pair->sampling >= 0.25 * pair->exec_loops)
+               pair->sampling_exit = 1;
 
        if (pair->exec_time_p < rem_p) {
                tst_res(TINFO,
                        "Exceeded execution time, requesting exit");
                exit = 1;
-
-               if (pair->sampling > 0) {
-                       tst_res(TWARN,
-                               "Still sampling, consider increasing
 LTP_TIMEOUT_MUL");
-               }
        }

        if (++pair->exec_loop > pair->exec_loops) {

--
Regards,
Li Wang

----- Original Message -----

Quoted 47 lines · Jan Stancek

Quoted 8 lines

Fixes #429

Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far.

This patch makes sense.

Quoted 35 lines

Signed-off-by Jan Stancek <[email protected]>

---
 include/tst_fuzzy_sync.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

include/tst_fuzzy_sync.h +2 -0

index 03f69b78bc82..0ca292c141c3 100644
@@ -162,6 +162,7 @@ struct tst_fzsync_pair {
         *
         * Defaults to 0.5 (~150 seconds with default timeout).
         */
+       float max_sampling_p;
        float exec_time_p;
        /** Internal; The test time remaining on tst_fzsync_pair_reset() */
        float exec_time_start;

@@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair
 *pair)
        CHK(avg_alpha, 0, 1, 0.25);
        CHK(min_samples, 20, INT_MAX, 1024);
        CHK(max_dev_ratio, 0, 1, 0.1);
+       CHK(max_sampling_p, 0, 1, 0.25);
        CHK(exec_time_p, 0, 1, 0.5);
        CHK(exec_loops, 20, INT_MAX, 3000000);

}

include/tst_fuzzy_sync.h +3 -0

@@ -582,9 +584,18 @@ static inline void tst_fzsync_wait_b(struct
 tst_fzsync_pair *pair)
 static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair)
 {
        int exit = 0;
+       float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start;
+
+       /* Limit amount of time spent on sampling */

Quoted 8 lines

include/tst_fuzzy_sync.h static inline void tst_fzsync_wait_b(struct · line 594

Quoted 4 lines

include/tst_fuzzy_sync.h +4 -0

+       if ((pair->max_sampling_p < rem_p)
+               && (pair->sampling > 0)) {
+               tst_res(TINFO, "stopping sampling at %d samples",
+                       pair->sampling);

pair->sampling is the number of samples left, so I think the prints might be more precise if calculate the 'pair->min_samples - pair->sampling'(or pair->exec_loop)?

Yes, I was lazy.

Quoted 12 lines

Quoted 11 lines

include/tst_fuzzy_sync.h +4 -2

+               pair->sampling = 0;
+               tst_fzsync_pair_info(pair);
+       }
 
-       if (pair->exec_time_p
-           < 1 - tst_timeout_remaining() / pair->exec_time_start) {
+       if (pair->exec_time_p < rem_p) {
                tst_res(TINFO,
                        "Exceeded execution time, requesting exit");

                exit = 1;

Quoted 3 lines

After involving max_sampling_p, here the warning prints will no longer be useful because it will never get 'pair->sampling > 0' when exec_time_p is exhausted.

You're right, "still sampling" warning can't be reached now.

In my first iteration I had it limited only to single CPU systems, so the warning was still possible on SMP.

I'll send v2.

Quoted 28 lines

Base on your patch I suggest that:

include/tst_fuzzy_sync.h +1 -1

@@ -590,7 +590,7 @@ static inline int tst_fzsync_run_a(struct
 tst_fzsync_pair *pair)
        if ((pair->max_sampling_p < rem_p)
                && (pair->sampling > 0)) {
                tst_res(TINFO, "stopping sampling at %d samples",
-                       pair->sampling);
+                       pair->exec_loop);
                pair->sampling = 0;
                tst_fzsync_pair_info(pair);

        }

include/tst_fuzzy_sync.h +0 -5

@@ -599,11 +599,6 @@ static inline int tst_fzsync_run_a(struct
 tst_fzsync_pair *pair)
                tst_res(TINFO,
                        "Exceeded execution time, requesting exit");
                exit = 1;
-
-               if (pair->sampling > 0) {
-                       tst_res(TWARN,
-                               "Still sampling, consider increasing
 LTP_TIMEOUT_MUL");
-               }
        }

Quoted 7 lines

        if (++pair->exec_loop > pair->exec_loops) {

======= Snip ========

Additionally, If we go this way to limit the sampling time, seems 'pair->min_samples' will become less meaning for fuzzy sync library,

True, but this applies only when sampling goes very slowly.

Quoted 2 lines

because we can just do sampling in a limited time and why we need the minimal samples?

Wouldn't we spend too much time sampling on fast systems without much gain?

My impression was we want to get out of sampling phase ASAP, so library can start inserting random delays.

Regards,
Jan

Quoted 123 lines

So what about using limited-time to replace min_samples? Maybe like:

include/tst_fuzzy_sync.h +6 -12

@@ -123,19 +123,14 @@ struct tst_fzsync_pair {
         */
        int delay;
        /**
-        *  Internal; The number of samples left or the sampling state.
+        *  Internal; The number of samples in the sampling phase.
         *
-        *  A positive value is the number of remaining mandatory
+        *  A positive value is the number of past mandatory
         *  samples. Zero or a negative indicate some other state.
         */
        int sampling;
-       /**
-        * The Minimum number of statistical samples which must be collected.
-        *
-        * The minimum number of iterations which must be performed before a
-        * random delay can be calculated. Defaults to 1024.
-        */
-       int min_samples;
+       /** Internal; Exit sampling phase if this value is 1 */
+       int sampling_exit;
        /**
         * The maximum allowed proportional average deviation.
         *

@@ -198,7 +193,6 @@ struct tst_fzsync_pair {
 static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
 {
        CHK(avg_alpha, 0, 1, 0.25);
-       CHK(min_samples, 20, INT_MAX, 1024);
        CHK(max_dev_ratio, 0, 1, 0.1);
        CHK(max_sampling_p, 0, 1, 0.25);
        CHK(exec_time_p, 0, 1, 0.5);

@@ -262,7 +256,8 @@ static void tst_fzsync_pair_reset(struct
 tst_fzsync_pair *pair,
        tst_init_stat(&pair->diff_ab);
        tst_init_stat(&pair->spins_avg);
        pair->delay = 0;
-       pair->sampling = pair->min_samples;
+       pair->sampling = 0;
+       pair->sampling_exit = 0;
 
        pair->exec_loop = 0;

@@ -295,7 +290,6 @@ static inline void tst_fzsync_stat_info(struct
 tst_fzsync_stat stat,
  */
 static void tst_fzsync_pair_info(struct tst_fzsync_pair *pair)
 {
-       tst_res(TINFO, "loop = %d", pair->exec_loop);
        tst_fzsync_stat_info(pair->diff_ss, "ns", "start_a - start_b");
        tst_fzsync_stat_info(pair->diff_sa, "ns", "end_a - start_a");

        tst_fzsync_stat_info(pair->diff_sb, "ns", "end_b - start_b");

include/tst_fuzzy_sync.h +3 -0

@@ -450,6 +444,9 @@ static void tst_fzsync_pair_update(struct
 tst_fzsync_pair *pair)
        float alpha = pair->avg_alpha;
        float per_spin_time, time_delay, dev_ratio;
 
+       if (!pair->sampling_exit)
+               ++pair->sampling;
+
        dev_ratio = (pair->diff_sa.dev_ratio
                     + pair->diff_sb.dev_ratio

+ pair->diff_ab.dev_ratio

include/tst_fuzzy_sync.h +5 -3

@@ -465,11 +462,13 @@ static void tst_fzsync_pair_update(struct
 tst_fzsync_pair *pair)
                tst_upd_diff_stat(&pair->diff_ab, alpha,
                                  pair->a_end, pair->b_end);
                tst_upd_stat(&pair->spins_avg, alpha, pair->spins);
-               if (pair->sampling > 0 && --pair->sampling == 0) {
+               if (pair->sampling_exit && pair->sampling > 0) {
                        tst_res(TINFO,
-                               "Minimum sampling period ended,
 deviation ratio = %.2f",
-                               dev_ratio);
+                               "Sampling period ended, total samples = %d,"
+                               "deviation ratio = %.2f",
+                               pair->sampling, dev_ratio);
                        tst_fzsync_pair_info(pair);
+                       pair->sampling = 0;
                }

        } else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1)
        {
                per_spin_time = fabsf(pair->diff_ab.avg) /
                pair->spins_avg.avg;

include/tst_fuzzy_sync.h +3 -12

@@ -587,23 +586,14 @@ static inline int tst_fzsync_run_a(struct
 tst_fzsync_pair *pair)
        float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start;
 
        /* Limit amount of time spent on sampling */
-       if ((pair->max_sampling_p < rem_p)
-               && (pair->sampling > 0)) {
-               tst_res(TINFO, "stopping sampling at %d samples",
-                       pair->sampling);
-               pair->sampling = 0;
-               tst_fzsync_pair_info(pair);
-       }
+       if ((pair->max_sampling_p < rem_p && pair->sampling > 0)
+                       || pair->sampling >= 0.25 * pair->exec_loops)
+               pair->sampling_exit = 1;
 
        if (pair->exec_time_p < rem_p) {
                tst_res(TINFO,
                        "Exceeded execution time, requesting exit");
                exit = 1;
-
-               if (pair->sampling > 0) {
-                       tst_res(TWARN,
-                               "Still sampling, consider increasing
 LTP_TIMEOUT_MUL");
-               }
        }

        if (++pair->exec_loop > pair->exec_loops) {

--
Regards,
Li Wang

Wouldn't we spend too much time sampling on fast systems without much gain?

My impression was we want to get out of sampling phase ASAP, so library can start inserting random delays.

Fixes #429

Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far.

Signed-off-by Jan Stancek <[email protected]>

Fixes #429

Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far.

Signed-off-by Jan Stancek <[email protected]>

---
 include/tst_fuzzy_sync.h | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

Changes in v2:
- drop 'still sampling' warning
- use exec_loop in 'stopping sampling at' message

include/tst_fuzzy_sync.h +12 -1

index 03f69b78bc82..d572461c24b6 100644
@@ -162,6 +162,7 @@ struct tst_fzsync_pair {
 	 *
 	 * Defaults to 0.5 (~150 seconds with default timeout).
 	 */
+	float max_sampling_p;
 	float exec_time_p;
 	/** Internal; The test time remaining on tst_fzsync_pair_reset() */
 	float exec_time_start;

@@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
 	CHK(avg_alpha, 0, 1, 0.25);
 	CHK(min_samples, 20, INT_MAX, 1024);
 	CHK(max_dev_ratio, 0, 1, 0.1);
+	CHK(max_sampling_p, 0, 1, 0.25);
 	CHK(exec_time_p, 0, 1, 0.5);
 	CHK(exec_loops, 20, INT_MAX, 3000000);
 }

@@ -582,17 +584,21 @@ static inline void tst_fzsync_wait_b(struct tst_fzsync_pair *pair)
 static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair)
 {
 	int exit = 0;
+	float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start;
+
+	/* Limit amount of time spent on sampling */
+	if ((pair->max_sampling_p < rem_p)
+		&& (pair->sampling > 0)) {
+		tst_res(TINFO, "stopping sampling at %d samples",
+			pair->exec_loop);
+		pair->sampling = 0;
+		tst_fzsync_pair_info(pair);
+	}
 
-	if (pair->exec_time_p

Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far.