[Mesa-dev] [PATCH 00/21] Reduce ir_variable memory usage

[Tapani Pälli]

Just one nitpick about spec for patch 4 (feel free to ignore), otherwise

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

(I also run Piglit quick, no regressions)


On 05/28/2014 05:48 AM, Ian Romanick wrote:
> This series reduces the memory usage of ir_variable quite significantly.
>
> The first couple patches add a mechanism to determine the amount of
> memory used by any kind of IR object.  This is used to collect the data
> that is shown in the commit messages through the series.
>
> Most of the rest of the patches rearrange data or store things in
> smaller fields.  The two interesting "subseries" are:
>
> Patches 9 through 15 and 20 through 21: Store short variable names in
> otherwise "dead" space in the base class.  I didn't rebase these patches
> to all be together because I didn't want to re-collect all the data. :)
> A small amount more savings could be had here, but in the test case at
> hand, it didn't appear worth the effort.  Adding
>
> diff --git a/src/glsl/ir_memory_usage.cpp b/src/glsl/ir_memory_usage.cpp
> index f122635..2aead7c 100644
> --- a/src/glsl/ir_memory_usage.cpp
> +++ b/src/glsl/ir_memory_usage.cpp
> @@ -67,6 +67,9 @@ ir_memory_usage::visit(ir_variable *ir)
>     if (ir->name != (char *) ir->padding)
>        this->s.variable_name_usage += strlen(ir->name) + 1 + ralloc_header_size;
>  
> +   if (ir->name != (char *) ir->padding && ir->data.mode == ir_var_temporary)
> +      printf("IR MEM: %s\n", ir->name);
> +
>     return visit_continue;
>  }
>  
> may show some other possibilities.
>
> Patches 16 through 18: Store two fields that are never both used in the
> same location.
>
> Here's the punchline.  In a trimmed trace from dota2 on 32-bit,
> ir_variable accounts for ~5.5MB before this series.  After this series,
> it accounts for only ~4.5MB.
>
> Before: IR MEM: variable usage / name / total: 4955496 915817 5871313
> After:  IR MEM: variable usage / name / total: 4118280 644100 4762380
>
> I would love to see before / after data for a full run of dota2 with all
> the shaders compiled. :)
>
> This is also available in the ir_variable-diet-v2 branch of my fdo
> tree.  The ir_variable-diet branch contains a false start.  I tried to
> move a bunch of fields that are only used for shader interface variables
> (e.g., uniforms or varyings) to a dynamically allocated structure.  At
> least on my test case, the added ralloc overhead made that a loss.  It
> may be possible to try a similar techinique by subclassing ir_varible,
> but I think that will be a lot of work.  The biggest annoyance is that
> when ast_to_hir allocates an ir_variable, it doesn't yet know what it
> will be... and changes the ir_variable_data::mode after allocation.
>
> As a side note, pahole is a really useful utility, but it lies a little
> bit on C++ objects.  It will say things like:
>
> class ir_rvalue : public ir_instruction {
> public:
>
>         /* class ir_instruction      <ancestor>; */      /*     0     0 */
>
>         /* XXX 16 bytes hole, try to pack */
>
>         const class glsl_type  *   type;                 /*    16     4 */
>
>         /* size: 20, cachelines: 1, members: 2 */
>         /* sum members: 4, holes: 1, sum holes: 16 */
>         /* last cacheline: 20 bytes */
> };
>
> I've trimmed out all the methods and other noise.  It says there's this
> 16-byte "hole."  Notice sizeof(ir_instruction) is 16 bytes and the total
> size of ir_rvalue is 20 bytes.  This 16-byte hole is the storage for the
> base class members! :) Calling it a hole is a bit misleading.  This also
> sent me down a false path, but, thankfully, not for too long.
>
> Cc: Eero Tamminen <eero.t.tamminen at intel.com>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev