[Mesa-dev] mesa: Reduce libGL.so binary size by about 15%
Arlie Davis
arlied at google.com
Tue Sep 22 13:26:19 PDT 2015
Any comments on v2 of this patch?
On Thu, Sep 17, 2015 at 3:19 PM, Arlie Davis <arlied at google.com> wrote:
> Ok, here's v2 of the change, with the suggested edits.
>
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150922/88164da9/attachment-0001.html>
More information about the mesa-dev
mailing list