[Mesa-dev] [PATCH] mesa/mtypes: repack gl_texture_object.

[Eero Tamminen]

Hi,

On 03.09.2017 14:22, Thomas Helland wrote:
> 2017-09-03 13:18 GMT+02:00 Dave Airlie <airlied at gmail.com>:
>> From: Dave Airlie <airlied at redhat.com>
>>
>> reduces size from 1144 to 1128.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>   src/mesa/main/mtypes.h | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index d44897b..3d68a6d 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -1012,7 +1012,6 @@ struct gl_texture_object
>>      struct gl_sampler_object Sampler;
>>
>>      GLenum DepthMode;           /**< GL_ARB_depth_texture */
>> -   bool StencilSampling;       /**< Should we sample stencil instead of depth? */
>>
>>      GLfloat Priority;           /**< in [0,1] */
>>      GLint BaseLevel;            /**< min mipmap level, OpenGL 1.2 */
>> @@ -1033,12 +1032,17 @@ struct gl_texture_object
>>      GLboolean Immutable;        /**< GL_ARB_texture_storage */
>>      GLboolean _IsFloat;         /**< GL_OES_float_texture */
>>      GLboolean _IsHalfFloat;     /**< GL_OES_half_float_texture */
>> +   bool StencilSampling;       /**< Should we sample stencil instead of depth? */
>> +   bool HandleAllocated;       /**< GL_ARB_bindless_texture */
>>
> 
> Maybe we could use "pragma pack" here instead?


Structure re-organization (including changing their types) is done for 
three reasons:

* Portability between systems (based on their minimum alignment rules)

* Saving memory by minimizing structure sizes

* Improving performance by:
   - getting structures to fit better to caches
   - moving members that are used together to same cacheline


AFAIK there are three main reasons to use pragma pack on top of that:

* Legacy compatibility for non-portable ABIs (e.g. DOS/Windows ones) 
when members have to be at specific, non-optimal positions and alignments

* When a commonly used structure takes so much memory in total that 
program can run out of memory, and packing is only way to reduce it further

* Squeezing certain members to same cacheline after *profiling has 
shown* that them not being there is actually a problem


Issues that you might get from packing:

* On some platforms (at least older ARMs), CPU not guaranteeing atomic 
accesses if variable crosses page boundary (which could happen when you 
forbid compiler from aligning it naturally)

* Compiler optimizations alignment expectations if you pass parts of the 
structure elsewhere without telling compiler that it's not properly aligned


> I'm debating with myself whether or not moving this
> bool away from the rest of the bindless_texture related
> variables is worth saving the few bytes.

For performance reasons, it's good to keep things that are used 
together, also together in memory (structure), but granularity for that 
is pretty small... (cache line size, aligned)


	- Eero

>>      GLuint MinLevel;            /**< GL_ARB_texture_view */
>>      GLuint MinLayer;            /**< GL_ARB_texture_view */
>>      GLuint NumLevels;           /**< GL_ARB_texture_view */
>>      GLuint NumLayers;           /**< GL_ARB_texture_view */
>>
>> +   /** GL_EXT_memory_object */
>> +   GLenum TextureTiling;
>> +
>>      /** Actual texture images, indexed by [cube face] and [mipmap level] */
>>      struct gl_texture_image *Image[MAX_FACES][MAX_TEXTURE_LEVELS];
>>
>> @@ -1057,13 +1061,9 @@ struct gl_texture_object
>>      /** GL_ARB_shader_image_load_store */
>>      GLenum ImageFormatCompatibilityType;
>>
>> -   /** GL_EXT_memory_object */
>> -   GLenum TextureTiling;
>> -
>>      /** GL_ARB_bindless_texture */
>>      struct util_dynarray SamplerHandles;
>>      struct util_dynarray ImageHandles;
>> -   bool HandleAllocated;
>>   };
>>
>>
>> --
>> 2.9.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>