[Intel-gfx] [PATCH i-g-t 2/4] scripts/trace.pl: Sort order

[John Harrison]

On 1/22/2018 2:19 AM, Tvrtko Ursulin wrote:
>
> On 20/01/2018 00:24, John.C.Harrison at Intel.com wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> Add an extra level to the databse key sort so that the ordering is
>> deterministic. If the time stamp matches, it now compares the key
>> itself as well (context/seqno). This makes it much easier to determine
>> if a change has actually broken anything. Previously back to back runs
>> with no changes could still produce different output, especially when
>> adding extra debug output during the calculations.
>
> Makes sense. I guess I never expected ns resolution time stamps to be 
> different.

Computers are fast - ns is slow!

>
>> As the comparison test is now more than a single equation, moved it
>> out into a separate sort function.
>>
>> Signed-off-by: John Harrison <John.C.Harrison at intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   scripts/trace.pl | 26 ++++++++++++++++++++++----
>>   1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/trace.pl b/scripts/trace.pl
>> index 7b8a920e..cf841b7e 100755
>> --- a/scripts/trace.pl
>> +++ b/scripts/trace.pl
>> @@ -540,7 +540,25 @@ my (%submit_avg, %execute_avg, %ctxsave_avg);
>>   my $last_ts = 0;
>>   my $first_ts;
>>   -my @sorted_keys = sort {$db{$a}->{'start'} <=> $db{$b}->{'start'}} 
>> keys %db;
>> +sub sortStart {
>> +    my $as = $db{$a}->{'start'};
>> +    my $bs = $db{$b}->{'start'};
>> +
>> +    return $as <=> $bs if($as ne $bs);
>
> In the spirit of the last round of optimising work, I would check if 
> this is the most optimal way to write this. Perhaps it would be better 
> with a single comparison on this line. And not mixing string and 
> numerical context? Like:
>
>     my $val = $as <=> $bs;
>
>     $val = $a cmp $b if $val == 0;
>
>     return $val;
>
> ?
Six of one, half a dozen of another. Again, I would assume the compiler 
would be smart enough to make both versions equal but I'm happy to go 
with your version if you feel it is better.

>
>> +
>> +    return $a cmp $b;
>> +}
>> +
>> +sub sortQueue {
>> +    my $as = $db{$a}->{'queue'};
>> +    my $bs = $db{$b}->{'queue'};
>> +
>> +    return $as <=> $bs if($as ne $bs);
>> +
>> +    return $a cmp $b;
>> +}
>> +
>> +my @sorted_keys = sort sortStart keys %db;
>>   my $re_sort = 0;
>>   die "Database changed size?!" unless scalar(@sorted_keys) == 
>> $keyCount;
>>   @@ -589,9 +607,9 @@ foreach my $key (@sorted_keys) {
>>       $ctxsave_avg{$ring} += $db{$key}->{'end'} - $db{$key}->{'notify'};
>>   }
>>   - at sorted_keys = sort {$db{$a}->{'start'} <=> $db{$b}->{'start'}} 
>> keys %db if $re_sort;
>> + at sorted_keys = sort sortStart keys %db if $re_sort;
>>   -foreach my $ring (keys %batch_avg) {
>> +foreach my $ring (sort keys %batch_avg) {
>>       $batch_avg{$ring} /= $batch_count{$ring};
>>       $batch_total_avg{$ring} /= $batch_count{$ring};
>>       $submit_avg{$ring} /= $batch_count{$ring};
>> @@ -831,7 +849,7 @@ print <<ENDHTML;
>>   ENDHTML
>>     my $i = 0;
>> -foreach my $key (sort {$db{$a}->{'queue'} <=> $db{$b}->{'queue'}} 
>> keys %db) {
>> +foreach my $key (sort sortQueue keys %db) {
>>       my ($name, $ctx, $seqno) = ($db{$key}->{'name'}, 
>> $db{$key}->{'ctx'}, $db{$key}->{'seqno'});
>>       my ($queue, $start, $notify, $end) = ($db{$key}->{'queue'}, 
>> $db{$key}->{'start'}, $db{$key}->{'notify'}, $db{$key}->{'end'});
>>       my $submit = $queue + $db{$key}->{'submit-delay'};
>>
>
> Rest looks OK.
>
> Regards,
>
> Tvrtko