Wed, Jan 13, 2021 at 11:01:37AM CET, jtluka(a)redhat.com wrote:
Tue, Jan 12, 2021 at 03:26:01PM CET, olichtne(a)redhat.com wrote:
>On Thu, Jan 07, 2021 at 02:52:32PM +0100, Jan Tluka wrote:
>> Instead of using the whole set of samples from the measurements where
>> some of the samples may be irrelevant since each measurement can start
>> at different time, an aligned set of samples is generated to report and
>> evaluate.
>>
>> The alignment is done on all measurements done within an iteration.
>> A maximum timestamp of the first samples and the minimum timestamp of
>> the last samples of each measurement is retrieved from the measurements.
>> A copy of RecipeResults data with any samples not aligned within the
>> timestamp range removed is generated and used for reporting and evaluation.
>>
>> Signed-off-by: Jan Tluka <jtluka(a)redhat.com>
>> ---
>> lnst/RecipeCommon/Perf/Recipe.py | 34 ++++++++++++++++++++++++++++++--
>> 1 file changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/lnst/RecipeCommon/Perf/Recipe.py b/lnst/RecipeCommon/Perf/Recipe.py
>> index 4aaa704b..b57f33da 100644
>> --- a/lnst/RecipeCommon/Perf/Recipe.py
>> +++ b/lnst/RecipeCommon/Perf/Recipe.py
>> @@ -64,6 +64,35 @@ class RecipeResults(object):
>> aggregated_results, new_results)
>> self._aggregated_results[measurement] = aggregated_results
>>
>> + @property
>> + def time_aligned_results(self):
>> + dummy = list(self._results.keys())[0]
>> + iterations = len(self._results[dummy])
>
>This should just be:
>
> iterations = self.recipe_conf.iterations
>
Much better!
>> +
>> + timestamps = []
>> + for i in range(0, iterations):
>
>Cosmetic fyi, if the first parameter is 0 you can skip it -
>range(iterations) is the same.
>
I'll change this in v2.
>> + results_group = [measurement_result
>> + for iteration_results in self.results.values()
>> + for measurement_result in iteration_results[i]]
>
>Shouldn't this just be this?
>
> [
> measurement_results[iteration]
> for measurement_results in self.results.values()
> ]
>
>
>I reversed the variable names because, self.results should be:
>measurement1: [iteration1, iteration2, ...]
>measurement2: [iteration1, iteration2, ...]
>
>so self.results.values() should be:
>[
> [iteration1, iteration2, ...],
> [iteration1, iteration2, ...],
> ...
>]
>
>And I think the item in this list - "[iteration1, iteration2, ...]" is
>measurement_results list. Then the [i] selection is the actual
>iteration.
>
>I think this fits more wrt. variable naming, what do you think?
>
>
Yes, you're right. Much simpler. I'll update this and will retest just
to be sure, but it makes perfect sense with your notes on the data structure.
Actually, no. I've tested with this change and I'm getting an exception
when accessing the start_timestamp of the resulting object. And the
object is now a list which is incorrect, it should be an instance of
BaseMeasurementResults.
The thing is that each measurement:measurement_result is a LIST of
MeasurementResults instances, so I need to dive one level deeper which
was the original implementation.
To show that on your example:
>so self.results.values() should be:
>[
> [iteration1, iteration2, ...],
> [iteration1, iteration2, ...],
> ...
>]
That is correct, but each iteration here is [ cpu_result1, cpu_result2] ] and
that applies to FlowMeasurement as well. So, I need to compile measurements
in both lists of the CPUStatMeasurement and FlowMeasurement.
Maybe I could improve the variable naming in the list comprehension to
make it more readable.
>> +
>> + timestamps.append((
>> + max([res.start_timestamp for res in results_group]),
>> + min([res.end_timestamp for res in results_group])
>> + ))
>> +
>> + aligned_recipe_results = RecipeResults(self._recipe_conf)
>> + for measurement, measurement_results in self.results.items():
>
>see, you name it measurement_results here too :)
>
>> + for i, result in enumerate(measurement_results):
>
>I would rename the "result" to "measurement_iteration" here
>
Ack, and I will also rename the sub_result to result.
Thanks for review. I'll send v2 soon.
>> + aligned_measurement_results = []
>> + for sub_result in result:
>> + aligned_measurement_result =
sub_result.align_data(timestamps[i][0], timestamps[i][1])
>> +
aligned_measurement_results.append(aligned_measurement_result)
>> +
>> + aligned_recipe_results.add_measurement_results(measurement,
aligned_measurement_results)
>> +
>> + return aligned_recipe_results
>> +
>> +
>> class Recipe(BasePerfTestTweakMixin, BasePerfTestIterationTweakMixin,
BaseRecipe):
>> def perf_test(self, recipe_conf):
>> results = RecipeResults(recipe_conf)
>> @@ -100,9 +129,10 @@ class Recipe(BasePerfTestTweakMixin,
BasePerfTestIterationTweakMixin, BaseRecipe
>> self.add_result(True, "\n".join(description))
>>
>> def perf_report_and_evaluate(self, results):
>> - self.perf_report(results)
>> + aligned_results = results.time_aligned_results
>>
>> - self.perf_evaluate(results)
>> + self.perf_report(aligned_results)
>> + self.perf_evaluate(aligned_results)
>>
>> def perf_report(self, recipe_results):
>> if not recipe_results:
>> --
>> 2.26.2
>> _______________________________________________
>> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
>> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
>> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>> List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...
_______________________________________________
LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...