On 14.06.2017 10:05, Michel Dänzer wrote:
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.
Well, if (*ptr)->next is _not_ destroyed, then the new code will set
(*ptr)->next->next to NULL anyway, while the old code would not have
modified it.
Come to think of it, the new code should really break out of the loop
once pipe_reference_described returns false. Maybe:
do {
struct pipe_resource *next = old_tex->next;
old_tex->screen->resource_destroy(old_tex->screen, old_tex);
old_tex = next;
} while (old_tex && pipe_reference_described(&old_tex->reference, NULL,
...));
How does that sound?
Cheers,
Nicolai
Let me know of you spot another difference in behaviour before and after
the patch.
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
