[Intel-gfx] [PATCH i-g-t 1/4] scripts/trace.pl: More hash key optimisations

[John Harrison]

On 1/22/2018 2:14 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>
>>
>> Cache the key count value rather than querying the hash every time.
>
> This actually makes a difference? Just curious, I would have assumed 
> Perl would know the size of it's arrays but maybe the implementation 
> is stupid...

Actually, I'm not sure it does anymore. I thought it did but I later 
decided that the change was actually just run to run noise. However, I 
already had the patch and I think it makes the code at least look 
simpler. IMHO, '$key_count' is easier to read than 'scalar(keys(%db))' 
and it is obviously trivial rather than relying on the compiler to be smart.

>
> Btw I was using Devel::NTYProf in HTML output mode to profile my 
> changes. It is quite easy to use and provided all the info I needed.
>
>> Also assert that the database does not magically change size after the
>> fixups.
>>
>> Signed-off-by: John Harrison <John.C.Harrison at intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   scripts/trace.pl | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/trace.pl b/scripts/trace.pl
>> index cb93900d..7b8a920e 100755
>> --- a/scripts/trace.pl
>> +++ b/scripts/trace.pl
>> @@ -508,7 +508,9 @@ foreach my $key (keys %db) {
>>   }
>>     # Fix up incompletes
>> -foreach my $key (keys %db) {
>> +my @keys = keys(%db);
>
> This array is unused except for the query below so I'd suggest to not 
> have it.
>
>> +my $keyCount = scalar(@keys);
>
> About came case.. I won't complain too the max, since I'm happy you 
> are finding the tool useful and improving it, but it would be good to 
> stay within the same style of variable naming or we'll have a 
> mish-mash of styles which won't help readability.

Oops, I missed that. Habit and too many different style guides in too 
many different projects! I'll change it to $key_count instead.


>
> Regards,
>
> Tvrtko
>
>> +foreach my $key (@keys) {
>>       next unless exists $db{$key}->{'incomplete'};
>>         # End the incomplete batch at the time next one starts
>> @@ -522,7 +524,7 @@ foreach my $key (keys %db) {
>>           $next_key = db_key($ring, $ctx, $seqno + $i);
>>           $i++;
>>       } until ((exists $db{$next_key} and not exists 
>> $db{$next_key}->{'incomplete'})
>> -         or $i > scalar(keys(%db)));  # ugly stop hack
>> +         or $i > $keyCount);  # ugly stop hack
>>         if (exists $db{$next_key}) {
>>           $db{$key}->{'notify'} = $db{$next_key}->{'end'};
>> @@ -540,6 +542,7 @@ my $first_ts;
>>     my @sorted_keys = sort {$db{$a}->{'start'} <=> 
>> $db{$b}->{'start'}} keys %db;
>>   my $re_sort = 0;
>> +die "Database changed size?!" unless scalar(@sorted_keys) == $keyCount;
>>     foreach my $key (@sorted_keys) {
>>       my $ring = $db{$key}->{'ring'};
>> @@ -565,7 +568,7 @@ foreach my $key (@sorted_keys) {
>>           do {
>>               $next_key = db_key($ring, $ctx, $seqno + $i);
>>               $i++;
>> -        } until (exists $db{$next_key} or $i > scalar(keys(%db)));  
>> # ugly stop hack
>> +        } until (exists $db{$next_key} or $i > $keyCount); # ugly 
>> stop hack
>>             # 20us tolerance
>>           if (exists $db{$next_key} and $db{$next_key}->{'start'} < 
>> $start + 20) {
>>