Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 30 Jan 2012 19:05:11 +0000
From:      David Chisnall <theraven@FreeBSD.org>
To:        David Schultz <das@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r227753 - in head: contrib/gdtoa include lib/libc/gdtoa lib/libc/gen lib/libc/locale lib/libc/regex lib/libc/stdio lib/libc/stdlib lib/libc/stdtime lib/libc/string
Message-ID:  <318F6CAA-D37E-49E9-A147-E21DE803EFB7@FreeBSD.org>
In-Reply-To: <20120118190714.GA13375@zim.MIT.EDU>
References:  <201111201445.pAKEjgNR096676@svn.freebsd.org> <20120118190714.GA13375@zim.MIT.EDU>

next in thread | previous in thread | raw e-mail | index | archive | help
On 18 Jan 2012, at 19:07, David Schultz wrote:

> This patch appears to cause a large performance regression.  For
> example, I measured a 78% slowdown for strtol("    42", ...).

That's definitely worth taking a closer look at.  I think we can cache =
some things in TLS and avoid some pthread_getspecific calls.  The =
current code is the 'make it work' version.  The 'make it fast' version =
is planned...

> Furthermore, the resulting static binary for a trivial program
> goes from 7k to 303k, due to pulling in malloc, stdio, and all the
> pthread stubs. =20

That's not ideal, but I'm not sure if it's avoidable.  Is statically =
linking libc something people regularly do?

> Presumably the capabilities of the non-xlocale
> entry points aren't appreciably changed,

Well... actually they are.  All of them now use the per-thread locale if =
one is set, and only fall back to the global one if it isn't.  The =
behaviour is only unchanged if nothing in the program calls uselocale().

> so there ought to be a
> way to avoid the overhead for them.  Do you have any thoughts on this?

Yup.  A quick-and-dirty hack would be to add a flag that was set on the =
first call to uselocale() and to always use the global locale if this is =
not set.  That should remove a lot of the overhead in cases where no one =
uses the per-thread locales. =20

We can also probably store the locale in TLS, which (on platforms with =
fast TLS) should speed up the lookup a bit. =20

> Some more minor issues...
>=20
> It's also customary to document public APIs so that, for
> instance, `man printf_l' pulls up a page with the prototype,
> required #includes, and behavior.  Aliasing manpages with
> MLINKS as appropriate is fine; for instance, Darwin's manpages
> on these functions look like a good example to follow.

Yup, all of the foo_l manpages are missing.  They're on my TODO, unless =
any docs people want to get there first...

> Finally, I'm not usually one to be picky about style, but could
> you make a pass to clean things up a little bit to match the
> surrounding code, wrap multiline comments to 80 columns, etc?
> You've also added new copyright notices for one-line changes
> (e.g., stdio/vdprintf.c, gdtoa/machdep_ldis?.c) and multiple
> copyright notices in the same file (locale/collate.c), which
> could be cleaned up concurrently.

I'll take a look.

David=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?318F6CAA-D37E-49E9-A147-E21DE803EFB7>