[Mesa-dev] [PATCH v3] gallium/util: Break recursion in pipe_resource_reference

[Michel Dänzer]

On 14/06/17 04:42 PM, Nicolai Hähnle wrote:
> On 14.06.2017 05:39, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer at amd.com>
>>
>> It calling itself recursively prevented it from being inlined, resulting
>> in a copy being generated in every compilation unit referencing it. This
>> bloated the text segment of the Gallium mega-driver *_dri.so by ~4%,
>> and might also have impacted performance.
> 
> Did you update the size measurement?

Yes, since I measured ~4.3%, I figured it's better to go for
understatement than exaggeration after all. :)


>> diff --git a/src/gallium/auxiliary/util/u_inlines.h
>> b/src/gallium/auxiliary/util/u_inlines.h
>> index 6a3d5043cf..08326da03d 100644
>> --- a/src/gallium/auxiliary/util/u_inlines.h
>> +++ b/src/gallium/auxiliary/util/u_inlines.h
>> @@ -137,8 +137,19 @@ pipe_resource_reference(struct pipe_resource
>> **ptr, struct pipe_resource *tex)
>>        if (pipe_reference_described(&(*ptr)->reference, &tex->reference,
>>                                  
>> (debug_reference_descriptor)debug_describe_resource)) {
>> -      pipe_resource_reference(&old_tex->next, NULL);
>> +      struct pipe_resource *next = old_tex->next;
>> +
>>         old_tex->screen->resource_destroy(old_tex->screen, old_tex);
>> +
>> +      /* Avoid recursion, which would prevent inlining this function */
>> +      while (next) {
>> +         old_tex = next;
>> +         next = old_tex->next;
>> +         old_tex->next = NULL;
> 
> Is this really necessary? I don't think so, but if it is, it's probably
> necessary above as well.

The old_tex->next = NULL assignment is necessary here to complement the
pipe_reference_described call below it. It's not necessary above,
because the ->next chain only needs to be unreferenced when the "master"
resource is destroyed.

The behaviour is basically the same as with the recursive
pipe_resource_reference(&old_tex->next, NULL) call before: the next
pointers are left untouched unless the "master" resource is destroyed,
in which case the next pointers are set to NULL and the reference counts
of the corresponding sub-resources decremented (and the sub-resource
destroyed if it goes to 0). The only slight change is that the next
member of the "master" resource isn't set to NULL, but that doesn't
matter because it's getting destroyed anyway.

Let me know of you spot another difference in behaviour before and after
the patch.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer