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

[Dave Airlie]

On 6 September 2017 at 03:11, Marek Olšák <maraeo at gmail.com> wrote:
> On Tue, Sep 5, 2017 at 5:50 PM, Brian Paul <brianp at vmware.com> wrote:
>> On 09/04/2017 05:29 AM, Marek Olšák wrote:
>>>
>>> On Sun, Sep 3, 2017 at 1:18 PM, Dave Airlie <airlied at gmail.com> wrote:
>>>>
>>>> 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 */
>>>
>>>
>>> The patch looks good, but here are some ideas for future improvements:
>>>
>>> GLenum can be uint16_t everywhere, because GL doesn't set higher bits:
>>>
>>> typedef uint16_t GLenum16.
>>> s/GLenum/GLenum16/
>>>
>>>> -   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 */
>>>
>>>
>>> All bools can be 1 bit:
>>>
>>> bool x:1;
>>> GLboolean y:1;
>>>
>>> etc.
>>>
>>>>
>>>>      GLuint MinLevel;            /**< GL_ARB_texture_view */
>>>>      GLuint MinLayer;            /**< GL_ARB_texture_view */
>>>>      GLuint NumLevels;           /**< GL_ARB_texture_view */
>>>>      GLuint NumLayers;           /**< GL_ARB_texture_view */
>>>
>>>
>>> MinLevel, NumLevels can be ubyte (uint8_t). MinLayer, NumLayers can be
>>> ushort (uint16_t)... simply by considering the range of possible
>>> values.
>>
>>
>> There's lots of opportunities along these lines in gl_texture_image. And
>> since we often have many gl_texture_images per gl_texture_object, and we
>> often have many textures, it'll probably have considerable impact.  I've
>> suggested this in the past but never got around to working on it.
>>
>> I recall Eric Anholt mentioning a memory profiling tool that was helpful for
>> finding wasted space in structures, etc.  I don't recall the name right now.
>> Eric?
>
> Dave used pahole for this patch series too. It can't obviously suggest
> what I suggested above (like changing the types and bits).

Yup this was pahole, doing what Marek describes is definitely something
that can be done, but needs a lot more care and attention.

Replacing bool with unsigned :1 fields isn't always a win, as you then
have a mask/shift on the accesses so overall may end up slowing things
down, and increasing instruction count etc.

Dave.