This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] Add STATIC_GETENV macro.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Mon, 11 Nov 2013 10:45:10 +0100
- Subject: Re: [PATCH] Add STATIC_GETENV macro.
- Authentication-results: sourceware.org; auth=none
- References: <20131109103822 dot GA9173 at domone> <527F3B35 dot 7050108 at redhat dot com> <20131110085500 dot GA5991 at domone> <52801056 dot 1080404 at redhat dot com>
On Sun, Nov 10, 2013 at 06:01:42PM -0500, Carlos O'Donell wrote:
> On 11/10/2013 03:55 AM, OndÅej BÃlka wrote:
> > On Sun, Nov 10, 2013 at 02:52:21AM -0500, Carlos O'Donell wrote:
> >> On 11/09/2013 05:38 AM, OndÅej BÃlka wrote:
> >>> Hi,
> >>> This adds a STATIC_GETENV macro for avoiding possible getenv bottlenecks as
> >>> documented in 'Make getenv O(1)' thread.
> >>> As this is useful addition this could be a gnu extension.
> >>> An patch that converts internal getenv uses to STATIC_GETENV will
> >>> follow.
> >>> There is question if we should make STATIC_GETENV call a secure_getenv
> >>> by default or add STATIC_SECURE_GETENV.
> >>> This depends on if getenv calls in next patch could be replaced by
> >>> secure getenv or not.
> >>> So far both choices are possible.
> >>> * stdlib/stdlib.h (STATIC_GETENV): Add.
> >>> * manual/startup.texi (Environment Access): Document STATIC_GETENV.
> >>> * NEWS: Mention STATIC_GETENV.
> >> I like where this is going, but new functions like this will need
> >> quite a bit of discussion, and it isn't clear to me what uses users
> >> would use this where they wouldn't just cache the value themselves?
> > How many will? When writing code getenv/STATIC_GETENV choice is mostly
> > automatic, doing caching less so.
> > It involves adding static variable and choosing appropriate name, if
> > conditional and if you take standard as seriously as Rich using atomic
> > operations to set result.
> > Also when one want to refactoring with codebase containing twenty
> > getenvs then with STATIC_GETENV its running a script and checking which
> > parts makes sense.
> > As user it is first checking which parts make sense and then rewrite
> > needed parts which is a repetitive task so when one looks how much he
> > should do he gives up.
> Let me think about this for a while.
> I'm not happy with the name STATIC_GETENV, I wish we could express
> the behaviour rather than implementation as part of the name, but
> that's just my opinion e.g. GETENV_CACHED or something similar.
I am ok also with that.
One of future directions is add to library that detect performance
problem a check of how often is getenv called and issue a suggestion to
use GETENV_CACHED or so for callers where it happens more than 10 times.
> The other issue is that we must support users compiling programs
> with much older compilers than we support building glibc. When
> were the __sync* builtins added? Can you always rely on them?
> I thought they were being deprecated in favour of the atomic
> builtins? I think you need several ifdefs there to account for
> various ages of compilers.
This is bit unclear as they are mentioned in 4.1 documentation but they
are earlier. I did not dig exact date, a first mention of
compare_and_swap in git log is here so we could start from that.
Author: rth <rth@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Sun May 7 01:37:07 2000 +0000
As far as atomic builtins are concerned they are a spurious addition, a
check for sync builtin and compatibility code should suffice.
Or as this is new extension we could document minimal required version.