[Mesa-dev] mesa: Reduce libGL.so binary size by about 15%
Ian Romanick
idr at freedesktop.org
Tue Sep 22 15:55:24 PDT 2015
On 09/17/2015 03:19 PM, Arlie Davis wrote:
> Ok, here's v2 of the change, with the suggested edits.
So... I think this code is fine, and I admire the effort. I have a
couple concerns.
1. We have no way to test this, so it's quite possible something was broken.
2. This function is only used in the OSX builds. Jeremy is the
maintainer for those builds, so I've added him to the CC list.
For every non-OSX build, we should just stop linking
src/mapi/glapi/glapi_gentable.c. I thought maybe the X sever used it,
but I couldn't find any evidence of that.
If this is still a viable route, I have a few suggestions of follow-on
patches...
I guess this patch is
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
but I really think we need to get Jeremy's approval before pushing it.
> From 5f393faa058f453408dfc640eecae3fe6335dfed Mon Sep 17 00:00:00 2001
> From: Arlie Davis <arlied at google.com>
> Date: Tue, 15 Sep 2015 09:58:34 -0700
> Subject: [PATCH] This patch significantly reduces the size of the libGL.so
> binary. It does not change the (externally visible) behavior of libGL.so at
> all.
>
> gl_gentable.py generates a function, _glapi_create_table_from_handle.
> This function allocates a large dispatch table, consisting of 1300 or so
> function pointers, and fills this dispatch table by doing symbol lookups
> on a given shared library. Previously, gl_gentable.py would generate a
> single, very large _glapi_create_table_from_handle function, with a short
> cluster of lines for each entry point (function). The idiom it generates
> was a NULL check, a call to snprintf, a call to dlsym / GetProcAddress,
> and then a store into the dispatch table. Since this function processes
> a large number of entry points, this code is duplicated many times over.
>
> We can encode the same information much more compactly, by using a lookup
> table. The previous total size of _glapi_create_table_from_handle on x64
> was 125848 bytes. By using a lookup table, the size of
> _glapi_create_table_from_handle (and the related lookup tables) is reduced
> to 10840 bytes. In other words, this enormous function is reduced by 91%.
> The size of the entire libGL.so binary (measured when stripped) itself drops
> by 15%.
>
> So the purpose of this change is to reduce the binary size, which frees up
> disk space, memory, etc.
> ---
> src/mapi/glapi/gen/gl_gentable.py | 57 ++++++++++++++++++++++++++++-----------
> 1 file changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/src/mapi/glapi/gen/gl_gentable.py b/src/mapi/glapi/gen/gl_gentable.py
> index 1b3eb72..7cd475a 100644
> --- a/src/mapi/glapi/gen/gl_gentable.py
> +++ b/src/mapi/glapi/gen/gl_gentable.py
> @@ -113,6 +113,9 @@ __glapi_gentable_set_remaining_noop(struct _glapi_table *disp) {
> dispatch[i] = p.v;
> }
>
> +"""
> +
> +footer = """
> struct _glapi_table *
> _glapi_create_table_from_handle(void *handle, const char *symbol_prefix) {
> struct _glapi_table *disp = calloc(_glapi_get_dispatch_table_size(), sizeof(_glapi_proc));
> @@ -123,27 +126,28 @@ _glapi_create_table_from_handle(void *handle, const char *symbol_prefix) {
>
> if(symbol_prefix == NULL)
> symbol_prefix = "";
> -"""
>
> -footer = """
> - __glapi_gentable_set_remaining_noop(disp);
> -
> - return disp;
> -}
> -"""
> + /* Note: This code relies on _glapi_table_func_names being sorted by the
> + * entry point index of each function.
> + */
> + for (int func_index = 0; func_index < GLAPI_TABLE_COUNT; ++func_index) {
> + const char *name = _glapi_table_func_names[func_index];
> + void ** procp = &((void **)disp)[func_index];
>
> -body_template = """
> - if(!disp->%(name)s) {
> - void ** procp = (void **) &disp->%(name)s;
> - snprintf(symboln, sizeof(symboln), "%%s%(entry_point)s", symbol_prefix);
> + snprintf(symboln, sizeof(symboln), \"%s%s\", symbol_prefix, name);
> #ifdef _WIN32
> *procp = GetProcAddress(handle, symboln);
> #else
> *procp = dlsym(handle, symboln);
> #endif
> }
> + __glapi_gentable_set_remaining_noop(disp);
> +
> + return disp;
> +}
> """
>
> +
> class PrintCode(gl_XML.gl_print_base):
>
> def __init__(self):
> @@ -180,12 +184,33 @@ class PrintCode(gl_XML.gl_print_base):
>
>
> def printBody(self, api):
> - for f in api.functionIterateByOffset():
> - for entry_point in f.entry_points:
> - vars = { 'entry_point' : entry_point,
> - 'name' : f.name }
>
> - print body_template % vars
> + # Determine how many functions have a defined offset.
> + func_count = 0
> + for f in api.functions_by_name.itervalues():
> + if f.offset != -1:
> + func_count += 1
> +
> + # Build the mapping from offset to function name.
> + funcnames = [None] * func_count
> + for f in api.functions_by_name.itervalues():
> + if f.offset != -1:
> + if not (funcnames[f.offset] is None):
> + raise Exception("Function table has more than one function with same offset (offset %d, func %s)" % (f.offset, f.name))
> + funcnames[f.offset] = f.name
> +
> + # Check that the table has no gaps. We expect a function at every offset,
> + # and the code which generates the table relies on this.
> + for i in xrange(0, func_count):
> + if funcnames[i] is None:
> + raise Exception("Function table has no function at offset %d" % (i))
> +
> + print "#define GLAPI_TABLE_COUNT %d" % func_count
> + print "static const char * const _glapi_table_func_names[GLAPI_TABLE_COUNT] = {"
> + for i in xrange(0, func_count):
> + print " /* %5d */ \"%s\"," % (i, funcnames[i])
> + print "};"
> +
> return
>
>
>
>
>
>
> On 09/16/2015 07:07 AM, Matt Turner wrote:
>> On Tue, Sep 15, 2015 at 10:38 AM, Arlie Davis <arlied at google.com> wrote:
>>>
>>> Hello! I noticed an inefficiency in libGL.so, so I thought I'd take a
>>> stab at fixing it. This is my first patch submitted to mesa-dev, so
>>> if I'm doing anything dumb, let me know. I can't use git send-email,
>>> but I've formatted the patch using git format-patch, which should
>>> hopefully produce similar output. The patch text (below) describes
>>> the inefficiency and the improvement.
>>>
>>>
>>>
>>> From 0abde226eed1b9f6052193f36f6cdc060698f621 Mon Sep 17 00:00:00 2001
>>> From: Arlie Davis <arlied at google.com>
>>> Date: Tue, 15 Sep 2015 09:58:34 -0700
>>> Subject: [PATCH] This patch significantly reduces the size of the libGL.so
>>> binary. It does not change the (externally visible) behavior of libGL.so at
>>> all.
>>>
>>> gl_gentable.py generates a function, _glapi_create_table_from_handle.
>>> This function allocates a large dispatch table, consisting of 1300 or so
>>> function pointers, and fills this dispatch table by doing symbol lookups
>>> on a given shared library. Previously, gl_gentable.py would generate a
>>> single, very large _glapi_create_table_from_handle function, with a short
>>> cluster of lines for each entry point (function). The idiom it generates
>>> was a NULL check, a call to snprintf, a call to dlsym / GetProcAddress,
>>> and then a store into the dispatch table. Since this function processes
>>> a large number of entry points, this code is duplicated many times over.
>>>
>>> We can encode the same information much more compactly, by using a lookup
>>> table. The previous total size of _glapi_create_table_from_handle on x64
>>> was 125848 bytes. By using a lookup table, the size of
>>> _glapi_create_table_from_handle (and the related lookup tables) is reduced
>>> to 10840 bytes. In other words, this enormous function is reduced by 91%.
>>> The size of the entire libGL.so binary (measured when stripped) itself drops
>>> by 15%.
>>>
>>> So the purpose of this change is to reduce the binary size, which frees up
>>> disk space, memory, etc.
>>> ---
>>
>> Seems like a nice change. size lib/libGL.so.1.2.0 on my system shows
>>
>> text data bss dec hex filename
>> 604031 11360 2792 618183 96ec7 lib/libGL.so.1.2.0 before
>> 490751 21920 2792 515463 7dd87 lib/libGL.so.1.2.0 after
>>
>> Feel free to include that in the commit message.
>>
>>> src/mapi/glapi/gen/gl_gentable.py | 56 ++++++++++++++++++++++++++++-----------
>>> 1 file changed, 40 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/src/mapi/glapi/gen/gl_gentable.py b/src/mapi/glapi/gen/gl_gentable.py
>>> index 1b3eb72..2563b6b 100644
>>> --- a/src/mapi/glapi/gen/gl_gentable.py
>>> +++ b/src/mapi/glapi/gen/gl_gentable.py
>>> @@ -113,6 +113,9 @@ __glapi_gentable_set_remaining_noop(struct _glapi_table *disp) {
>>> dispatch[i] = p.v;
>>> }
>>>
>>> +"""
>>> +
>>> +footer = """
>>> struct _glapi_table *
>>> _glapi_create_table_from_handle(void *handle, const char *symbol_prefix) {
>>> struct _glapi_table *disp = calloc(_glapi_get_dispatch_table_size(), sizeof(_glapi_proc));
>>> @@ -123,27 +126,27 @@ _glapi_create_table_from_handle(void *handle, const char *symbol_prefix) {
>>>
>>> if(symbol_prefix == NULL)
>>> symbol_prefix = "";
>>> -"""
>>>
>>> -footer = """
>>> - __glapi_gentable_set_remaining_noop(disp);
>>> -
>>> - return disp;
>>> -}
>>> -"""
>>> + /* Note: This code relies on _glapi_table_func_names being sorted by the
>>> + entry point index of each function. */
>>
>> Mesa style puts the */ on its own line for multiline comments.
>>
>>> + for (int func_index = 0; func_index < GLAPI_TABLE_COUNT; ++func_index) {
>>> + const char* name = _glapi_table_func_names[func_index];
>>
>> * goes with the var name, not the type. That is, "char* " should be "char *"
>>
>>> + void ** procp = &((void **)disp)[func_index];
>>>
>>> -body_template = """
>>> - if(!disp->%(name)s) {
>>
>> We're removing the null check. Is that okay to do?
>>
>>> - void ** procp = (void **) &disp->%(name)s;
>>> - snprintf(symboln, sizeof(symboln), "%%s%(entry_point)s", symbol_prefix);
>>> + snprintf(symboln, sizeof(symboln), \"%s%s\", symbol_prefix, name);
>>> #ifdef _WIN32
>>> *procp = GetProcAddress(handle, symboln);
>>> #else
>>> *procp = dlsym(handle, symboln);
>>> #endif
>>> }
>>> + __glapi_gentable_set_remaining_noop(disp);
>>> +
>>> + return disp;
>>> +}
>>> """
>>>
>>> +
>>> class PrintCode(gl_XML.gl_print_base):
>>>
>>> def __init__(self):
>>> @@ -180,12 +183,33 @@ class PrintCode(gl_XML.gl_print_base):
>>>
>>>
>>> def printBody(self, api):
>>> - for f in api.functionIterateByOffset():
>>> - for entry_point in f.entry_points:
>>> - vars = { 'entry_point' : entry_point,
>>> - 'name' : f.name }
>>>
>>> - print body_template % vars
>>> + # Determine how many functions have a defined offset.
>>> + func_count = 0
>>> + for f in api.functions_by_name.itervalues():
>>> + if f.offset != -1:
>>> + func_count += 1
>>> +
>>> + # Build the mapping from offset to function name.
>>> + funcnames = [None] * func_count
>>> + for f in api.functions_by_name.itervalues():
>>> + if f.offset != -1:
>>> + if not (funcnames[f.offset] is None):
>>> + raise Exception("Function table has more than one function with same offset (offset %d, func %s)" % (f.offset, f.name))
>>> + funcnames[f.offset] = f.name
>>> +
>>> + # Check that the table has no gaps. We expect a function at every offset,
>>> + # and the code which generates the table relies on this.
>>> + for i in range(0, func_count):
>>
>> I don't know much python, but I think you want to use xrange instead of range.
>>
>>> + if funcnames[i] is None:
>>> + raise Exception("Function table has no function at offset %d" % (i))
>>> +
>>> + print "#define GLAPI_TABLE_COUNT %d" % func_count
>>> + print "static const char* const _glapi_table_func_names[GLAPI_TABLE_COUNT] = {"
>>
>> "char*" -> "char *"
>>
>>> + for i in range(0, func_count):
>>
>> xrange instead of range, I think.
>>
>>> + print " /* %5d */ \"%s\"," % (i, funcnames[i])
>>> + print "};"
>>> +
>>> return
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list