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.daen...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev