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

2012-01-30 Thread David Chisnall
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.  

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.  

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

 Some more minor issues...
 
 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___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


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

2012-01-30 Thread David Schultz
On Mon, Jan 30, 2012, David Chisnall wrote:
 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...

Sounds good; I look forward to it.

  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.  
 
 That's not ideal, but I'm not sure if it's avoidable.  Is statically linking 
 libc something people regularly do?

Aside from bde, probably not many.  This is definitely a
second-order concern.

FreeBSD has a set of statically linked binaries in /rescue for
situations where /lib gets screwed up.  Space is an issue there
because the root partition is historically sized quite small.

Embedded folks might also care, but I'll let them speak for
themselves.  I did get a request several years ago from an
embedded developer to unbreak the NO_FLOATING_POINT option in
libc, and you could imagine perhaps a NO_LOCALE option as well.

 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.  

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

I thought that's what thread_(get|set)_locale already did.
Actually, it's counterintuitive that it would be significantly
slower to access per-thread state than global state.  Any idea
why?  Maybe it says something about our pthread_getspecific()
implementation.  I will run the code through a profiler some
day, but I don't have the cycles right now.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


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

2012-01-30 Thread Ian Lepore
On Mon, 2012-01-30 at 16:26 -0500, David Schultz wrote:
 On Mon, Jan 30, 2012, David Chisnall wrote:
  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...
 
 Sounds good; I look forward to it.
 
   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.  
  
  That's not ideal, but I'm not sure if it's avoidable.  Is statically 
  linking libc something people regularly do?
 
 Aside from bde, probably not many.  This is definitely a
 second-order concern.
 
 FreeBSD has a set of statically linked binaries in /rescue for
 situations where /lib gets screwed up.  Space is an issue there
 because the root partition is historically sized quite small.
 
 Embedded folks might also care, but I'll let them speak for
 themselves.  I did get a request several years ago from an
 embedded developer to unbreak the NO_FLOATING_POINT option in
 libc, and you could imagine perhaps a NO_LOCALE option as well.
 

If locale support starts adding a lot of cycles or memory, then we
embedded folks might like a NO_LOCALE option.  100k here, 200k there,
and before you know it you're cursing some hardware designer who was
sure that you'd never use all of the 64MB of ram he generously gave you.

-- Ian


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


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

2012-01-30 Thread Steve Kargl
On Mon, Jan 30, 2012 at 04:26:31PM -0500, David Schultz wrote:
 
 FreeBSD has a set of statically linked binaries in /rescue for
 situations where /lib gets screwed up.  Space is an issue there
 because the root partition is historically sized quite small.

/rescue is a single binary created with crunchgen and
137 hard links.  There's also one 9K script.

BTW, I known of someone besides bde who creates statically
linked binaries; particularly when he's debugging and profiling
the his code. :-)
 
-- 
Steve
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


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

2012-01-29 Thread David Schultz
On Wed, Jan 18, 2012, David Schultz wrote:
 On Sun, Nov 20, 2011, David Chisnall wrote:
  Author: theraven
  Date: Sun Nov 20 14:45:42 2011
  New Revision: 227753
  URL: http://svn.freebsd.org/changeset/base/227753
  
  Log:
Implement xlocale APIs from Darwin, mainly for use by libc++.  This adds a
load of _l suffixed versions of various standard library functions that 
  use
the global locale, making them take an explicit locale parameter.  Also
adds support for per-thread locales.  This work was funded by the FreeBSD
Foundation.

Please test any code you have that uses the C standard locale functions!

Reviewed by:das (gdtoa changes)
Approved by:dim (mentor)
 
 This patch appears to cause a large performance regression.  For
 example, I measured a 78% slowdown for strtol(42, ...).
 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.  Presumably the capabilities of the non-xlocale
 entry points aren't appreciably changed, so there ought to be a
 way to avoid the overhead for them.  Do you have any thoughts on this?
 
 Some more minor issues...
 
 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.
 
 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.

Any plans to look into this?
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


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

2012-01-18 Thread David Schultz
On Sun, Nov 20, 2011, David Chisnall wrote:
 Author: theraven
 Date: Sun Nov 20 14:45:42 2011
 New Revision: 227753
 URL: http://svn.freebsd.org/changeset/base/227753
 
 Log:
   Implement xlocale APIs from Darwin, mainly for use by libc++.  This adds a
   load of _l suffixed versions of various standard library functions that use
   the global locale, making them take an explicit locale parameter.  Also
   adds support for per-thread locales.  This work was funded by the FreeBSD
   Foundation.
   
   Please test any code you have that uses the C standard locale functions!
   
   Reviewed by:das (gdtoa changes)
   Approved by:dim (mentor)

This patch appears to cause a large performance regression.  For
example, I measured a 78% slowdown for strtol(42, ...).
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.  Presumably the capabilities of the non-xlocale
entry points aren't appreciably changed, so there ought to be a
way to avoid the overhead for them.  Do you have any thoughts on this?

Some more minor issues...

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.

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.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org