[1/2] Don't declare xcb_req as static

Submitted by Adam Jackson on June 28, 2017, 3:45 p.m.

Details

Message ID 20170628154542.22001-1-ajax@redhat.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Adam Jackson June 28, 2017, 3:45 p.m.
Doing so forces the compiler to allocate storage for the symbol in
.data, which means 24 bytes of dirty data and a relocation per request
function.

   text	   data	    bss	    dec	    hex	filename
  88888	   7136	      8	  96032	  17720	src/.libs/libxcb-glx.so.before
  92432	    680	      8	  93120	  16bc0	src/.libs/libxcb-glx.so.after

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 src/c_client.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/c_client.py b/src/c_client.py
index 57de3fb..3b61f35 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -2319,7 +2319,7 @@  def _c_request_helper(self, name, void, regular, aux=False, reply_fds=False):
     dimension = count + 2
 
     _c('{')
-    _c('    static const xcb_protocol_request_t xcb_req = {')
+    _c('    const xcb_protocol_request_t xcb_req = {')
     _c('        .count = %d,', count)
     _c('        .ext = %s,', func_ext_global)
     _c('        .opcode = %s,', self.c_request_name.upper())

Comments

Eric Anholt June 28, 2017, 5:08 p.m.
Adam Jackson <ajax@redhat.com> writes:

> Doing so forces the compiler to allocate storage for the symbol in
> .data, which means 24 bytes of dirty data and a relocation per request
> function.
>
>    text	   data	    bss	    dec	    hex	filename
>   88888	   7136	      8	  96032	  17720	src/.libs/libxcb-glx.so.before
>   92432	    680	      8	  93120	  16bc0	src/.libs/libxcb-glx.so.after

Increasing the executed-code size by 4% here makes me nervous -- done
any performance comparison before and after?  I think that delta will
get buried under the overhead of xcb_send_request, but it would be nice
to be sure.

Is the problem just the relocations in that data?  Would pulling the
req->ext out of xcb_protocol_request and passing it as another parameter
of a new xcb_send_request variant also fix it, while letting us keep the
remainder of the xcb_protocol_request_t static?
Keith Packard June 28, 2017, 5:11 p.m.
Adam Jackson <ajax@redhat.com> writes:

> Doing so forces the compiler to allocate storage for the symbol in
> .data, which means 24 bytes of dirty data and a relocation per request
> function.

Yeah, no way to make it actually constant as it has a pointer to the
xcb_extension_t.

Reviewed-by: Keith Packard <keithp@keithp.com>
Keith Packard June 28, 2017, 5:28 p.m.
Eric Anholt <eric@anholt.net> writes:

> Increasing the executed-code size by 4% here makes me nervous

Overall memory usage in the system will be less as you get rid of the
data segment duplication in every process.

> Is the problem just the relocations in that data?

Yes, even if the xcb_extension_t is also const, you've got a relocation
to deal with.

> Would pulling the req->ext out of xcb_protocol_request and passing it
> as another parameter of a new xcb_send_request variant also fix it,
> while letting us keep the remainder of the xcb_protocol_request_t
> static?

Yes, it looks like that should work. I note that there's also a global
mutex lock for every extension request which would be nice to eliminate
at some point :-)