Just one nitpick about spec for patch 4 (feel free to ignore), otherwise
Reviewed-by: Tapani Pälli <tapani.pa...@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.tammi...@intel.com>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
