This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC][PATCH] LD_DEBUG option to measure init time
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Maria Guseva <m dot guseva at samsung dot com>, pinskia at gmail dot com
- Cc: libc-alpha at sourceware dot org, "'Yury Gribov'" <y dot gribov at samsung dot com>, v dot garbuzov at samsung dot com
- Date: Tue, 04 Aug 2015 14:55:41 -0400
- Subject: Re: [RFC][PATCH] LD_DEBUG option to measure init time
- Authentication-results: sourceware.org; auth=none
- References: <"027601d0cac7$952818c0$bf784a40$ at guseva"@samsung.com> <55BAF138 dot 1070904 at redhat dot com> <6908D9A3-3615-41A8-B8BC-20CCD871AD65 at gmail dot com> <55BAF4C8 dot 40200 at redhat dot com> <9526E1C6-ED9B-4688-A221-4DF3B8D048BA at gmail dot com> <55BAF98B dot 2000603 at redhat dot com> <"039401d0ced9$44783ca0$cd68b5e0$ at guseva"@samsung.com>
On 08/04/2015 01:16 PM, Maria Guseva wrote:
> I would like to add some generic timing to support platforms where HP time
> is unavailable. But it looks to me like separate patch which should affect
> other existing features like LD_DEBUG=statistics where HP_TIMING_NONAVAIL is
> checked as well.
I agree that two distinct patches are a good idea. However, I want to set the
expectation that for a complete implementation you must support both. I do
not consider this a complete solution until both HP and non-HP timings
are supported, either via a generic solution for HP or another interface.
> I think it might be some "nonhp-timing.h" header with equivalents of
> HP_TIMING_* macros redefined in generic approach. Thus it should entail not
> major changes in other glibc sources using them.
We already have a generic hp-timing.h e.g. sysdeps/generic/hp-timing.h.
See the comments in that file. A generic solution would use gettimeofday
and provide a generic implementation. However, the overhead of gettimeofday
might be sufficient that it ruins the value of the results. I don't think
it matters, I think a generic "works everywhere" solution would be best
(helps new ports) However, that patch should be distinct, and the community
should comment on it.
> Tested patch on x86_64 platform. On platforms without HP time new feature
> isn't used at all and isn't shown in LD_DEBUG=help.
Can you please test on one non-HP platform to show it still compiles and you
didn't make any mistakes there?
> +#if defined SHARED && ! defined HP_TIMING_NONAVAIL
Should use `HP_TIMING_NONAVAIL', never `defined HP_TIMING_NONAVAIL' since
all macros should always be defined to a value of 0 or 1.
Likewise everywhere else.
See: https://sourceware.org/glibc/wiki/Wundef
Overall the patch is looking really good. I don't have enough time to review
thoroughly until 2.23 branches. Please ping again after the development branch
opens.
Cheers,
Carlos.