date:20120906


On 09/05/12 23:51, Martin Sebor wrote:

On 09/05/2012 09:03 PM, Stefan Teleman wrote:

[...]
Agreed.

But: if the choice is between an implementation which [1] breaks ABI
and [2] performs 20% worse -- even in contrived test cases -- than
another implementation [2] which doesn't break ABI, and performs
better than the first one,  why would we even consider the first one?


Breaking the ABI is not an option (unless we rev the version).
But I'm not sure I understand what you think breaks the ABI.


I think Stefan is referring to adding a mutex member variable to the facet in 
question and breaking binary compatibility. If that is the case I have confused 
things when I suggested exactly that, earlier. A cursory read through the 
__rw_facet source shows that inherits from __rw_synchronized in MT builds, 
therefore each facet carries its own mutex member.

Liviu


We don't need to add a new mutex -- we can use the __rw_facet
member for the locking. Or did you mean something else?

Martin



--Stefan




--
And now I see with eye serene
The very pulse of the machine.

Re: New committers?

2012-09-06 Thread Jim Jagielski

As an ASF project? It's not going to happen.

On Sep 4, 2012, at 12:04 PM, Martin Sebor  wrote:

> On 09/02/2012 08:42 AM, Jim Jagielski wrote:
>> 
>> On Sep 2, 2012, at 12:02 AM, Martin Sebor  wrote:
>> 
>>> On 08/31/2012 02:38 PM, Liviu Nicoara wrote:
 My input below.
 
 On 08/31/12 09:42, Wojciech Meyer wrote:
> The two significant ones (as far as I can understand):
> 
> - as I heard from Christopher Bergström that it's hard to push the
>   stdcxx to FreeBSD ports repository (I can understand it and that
>   sounds pretty bad, if that's the case then the board should consider
>   re-licensing as advised; I agree in general it's a hard decision for
>   the board, but imagine the project would benefit, IANAL tho)
 
 Christopher's wishes and goals may be different from others'. I do not
 believe he has ulterior motives that would be detrimental to the rest of
 us but AFAICT he has not made a compelling argument. Even with one, it
 stretches the imagination what could possibly convince Apache to give up
 on STDCXX ownership.
>>> 
>>> Just a point of clarity: the ASF doesn't "own" stdcxx. They license
>>> it from Rogue Wave which still has the copyright. (Not that anyone
>>> there realizes it or would know what to do with it if they did.)
>>> IIUC, that's also why they can't relicense it under different terms.
>>> 
>> 
>> FWIW, the ASF never requires copyright assignment... Just a copyright
>> license to "reproduce, prepare derivative works of, publicly display,
>> publicly perform, sublicense, and distribute Your Contributions and
>> such derivative works."
>> 
>> Also, there is nothing in our bylaws or in the various license
>> agreements that *exclude* the ASF ever releasing code not under
>> the ALv2 (how could it? After all, that would prevent us from
>> ever being able to move to ALv3). Again, we could, if we wanted
>> to (which we never will, btw) actually make our code under the
>> GPLv2...
> 
> So what would it take to change the license to BSD as Christopher
> asks (IIUC)?
> 
> Martin
>

Re: STDCXX-1056 [was: Re: STDCXX forks]

On Thu, Sep 6, 2012 at 9:16 AM, Liviu Nicoara  wrote:

> I think Stefan is referring to adding a mutex member variable to the facet
> in question and breaking binary compatibility. If that is the case I have
> confused things when I suggested exactly that, earlier. A cursory read
> through the __rw_facet source shows that inherits from __rw_synchronized in
> MT builds, therefore each facet carries its own mutex member.

> On 09/05/12 23:51, Martin Sebor wrote:
>> We don't need to add a new mutex -- we can use the __rw_facet
>> member for the locking. Or did you mean something else?

A possible implementation using the __rw_facet mutex could look like this:

template 
inline string numpunct<_CharT>::grouping () const
{
if (!(_C_flags & _RW::__rw_gr)) {

numpunct* const __self = _RWSTD_CONST_CAST (numpunct*, this);

_RWSTD_MT_GUARD (__self->_C_mutex);

if (!(_C_flags & _RW::__rw_gr)) {

// [try to] get the grouping first (may throw)
// then set a flag to avoid future initializations
__self->_C_grouping  = do_grouping ();
__self->_C_flags|= _RW::__rw_gr;

}
}

return _C_grouping;
}

Except that it will not work. Because the __rw_facet mutex member is
being locked  in file ../src/facet.cpp in function
__rw_facet::_C_manage at line 366:

// acquire lock
_RWSTD_MT_STATIC_GUARD (_RW::__rw_facet);

This will deadlock because this is the mutex already locked by
std::numpunct::grouping().

I've already tested this with 3 compilers, and, it does indeed deadlock.

So yes, I did indeed mean something different. I meant adding another
mutex data member to the numpunct class.

--Stefan

-- 
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com

Re: STDCXX-1056 [was: Re: STDCXX forks]


On 09/06/2012 09:58 AM, Stefan Teleman wrote:

On Thu, Sep 6, 2012 at 9:16 AM, Liviu Nicoara  wrote:


I think Stefan is referring to adding a mutex member variable to the facet
in question and breaking binary compatibility. If that is the case I have
confused things when I suggested exactly that, earlier. A cursory read
through the __rw_facet source shows that inherits from __rw_synchronized in
MT builds, therefore each facet carries its own mutex member.



On 09/05/12 23:51, Martin Sebor wrote:

We don't need to add a new mutex -- we can use the __rw_facet
member for the locking. Or did you mean something else?


A possible implementation using the __rw_facet mutex could look like this:

template
inline string numpunct<_CharT>::grouping () const
{
 if (!(_C_flags&  _RW::__rw_gr)) {

 numpunct* const __self = _RWSTD_CONST_CAST (numpunct*, this);

 _RWSTD_MT_GUARD (__self->_C_mutex);

 if (!(_C_flags&  _RW::__rw_gr)) {

 // [try to] get the grouping first (may throw)
 // then set a flag to avoid future initializations
 __self->_C_grouping  = do_grouping ();
 __self->_C_flags|= _RW::__rw_gr;

 }
 }

 return _C_grouping;
}

Except that it will not work. Because the __rw_facet mutex member is
being locked  in file ../src/facet.cpp in function
__rw_facet::_C_manage at line 366:

// acquire lock
_RWSTD_MT_STATIC_GUARD (_RW::__rw_facet);


This locks a different mutex, one unrelated to __rw_facet::_C_mutex.

Look at the implementation of _RWSTD_MT_STATIC_GUARD() in rw/_defs.h:

  #  define _RWSTD_MT_STATIC_GUARD(type) \
typedef _RW::__rw_type _UniqueType;   \
_RWSTD_MT_CLASS_GUARD (_UniqueType)

  #  define _RWSTD_MT_CLASS_GUARD(type)   \
_RWSTD_MT_GUARD (_RW::__rw_get_static_mutex ((type*)0))

and then at __rw_get_static_mutex in rw/_mutex.h. It gets a static
("global") mutex from a "mutex factory" via template instantiation:

  __rw_static_mutex<__rw_facet>::_C_mutex;



This will deadlock because this is the mutex already locked by
std::numpunct::grouping().

I've already tested this with 3 compilers, and, it does indeed deadlock.


Something else must be locking the mutex then. I quickly looked
at __rw_get_punct (and __rw_get_numpunct) in punct.cpp but I
couldn't find any signs of the mutex being locked there. The
only thing I see being locked there is the global C locale. If
it's not the callee it has to be the caller.



So yes, I did indeed mean something different. I meant adding another
mutex data member to the numpunct class.


We can't (and shouldn't need to) add one. We need to be able to
make do with the existing member. That's what it's there for.

Martin



--Stefan

Re: New committers?

On 09/06/2012 07:22 AM, Jim Jagielski wrote:

As an ASF project? It's not going to happen.

Not necessarily as an ASF project. Christopher is interested
in moving the project somewhere else. See for example:

http://mail-archives.apache.org/mod_mbox/stdcxx-dev/201106.mbox/%3c4dfae25e.9040...@pathscale.com%3E

I would also like to know what the options are.

Martin

On Sep 4, 2012, at 12:04 PM, Martin Sebor wrote:

On 09/02/2012 08:42 AM, Jim Jagielski wrote:

On Sep 2, 2012, at 12:02 AM, Martin Sebor wrote:

On 08/31/2012 02:38 PM, Liviu Nicoara wrote:

My input below.

On 08/31/12 09:42, Wojciech Meyer wrote:

The two significant ones (as far as I can understand):

- as I heard from Christopher Bergström that it's hard to push the
stdcxx to FreeBSD ports repository (I can understand it and that
sounds pretty bad, if that's the case then the board should consider
re-licensing as advised; I agree in general it's a hard decision for
the board, but imagine the project would benefit, IANAL tho)

Christopher's wishes and goals may be different from others'. I do not
believe he has ulterior motives that would be detrimental to the rest of
us but AFAICT he has not made a compelling argument. Even with one, it
stretches the imagination what could possibly convince Apache to give up
on STDCXX ownership.

Just a point of clarity: the ASF doesn't "own" stdcxx. They license
it from Rogue Wave which still has the copyright. (Not that anyone
there realizes it or would know what to do with it if they did.)
IIUC, that's also why they can't relicense it under different terms.

FWIW, the ASF never requires copyright assignment... Just a copyright
license to "reproduce, prepare derivative works of, publicly display,
publicly perform, sublicense, and distribute Your Contributions and
such derivative works."

Also, there is nothing in our bylaws or in the various license
agreements that *exclude* the ASF ever releasing code not under
the ALv2 (how could it? After all, that would prevent us from
ever being able to move to ALv3). Again, we could, if we wanted
to (which we never will, btw) actually make our code under the
GPLv2...

So what would it take to change the license to BSD as Christopher
asks (IIUC)?

Martin

A question (or two) of procedure, etc.


What is the latest policy in what regards trivial fixes, e.g., the volatile 
qualifier for the max var in LIMITS.cpp we discussed earlier, etc.? It seems 
excessive to create a bug report for such issues.

Also, IIUC from reading previous discussions, forward and backward binary 
compatible changes go in 4.2.x, followed by merges to 4.3.x and trunk. Am I 
getting this right?

Also, besides the Linux, FreeBSD, Windows, Solaris builds hosted on Apache 
(Jenkins) is anybody building on HP-UX, AIX, etc.?

Thanks.

Liviu

Re: A question (or two) of procedure, etc.

2012-09-06 Thread Wojciech Meyer

Liviu Nicoara  writes:

> What is the latest policy in what regards trivial fixes, e.g., the
> volatile qualifier for the max var in LIMITS.cpp we discussed earlier,
> etc.? It seems excessive to create a bug report for such issues.

My advice based on some observations with other projects, is that in
these cases we go just go on and apply fix. Non invasive code quality
improvements over the codebase should be promoted not hindered. More
risky patches, should be discussed on the list, the ones that needs
either serious changes, attention, re-factoring, feature or fixes that
may break something should be logged in Jira.

So I vote for keeping the changes as lightweight as possible, and avoid
extra bureaucracy if it makes sense. This assumption is based that
developers here trust their selves, and run the tests often. I'm not
subscribed to the commit list here, but if I do - I usually follow
people's changes and assume that developers do read commit lists.

So the general consensus from my experience with other project was: not
sure - ask.

That's my experience, also I don't have full rights to express my
opinion right now about stdcxx.

> Also, IIUC from reading previous discussions, forward and backward
> binary compatible changes go in 4.2.x, followed by merges to 4.3.x and
> trunk. Am I getting this right?

Every project has certain branch strategy, I'm not sure about this so
maybe Martin can advice. I prefer to develop on trunk and cherry pick
to the other branches avoiding bulk merges (and that's in both
directions).

>
> Also, besides the Linux, FreeBSD, Windows, Solaris builds hosted on Apache 
> (Jenkins) is anybody building on HP-UX, AIX, etc.?
>
> Thanks.
>
> Liviu
>

--
Wojciech Meyer
http://danmey.org

Re: STDCXX-1056 [was: Re: STDCXX forks]

On Thu, Sep 6, 2012 at 12:16 PM, Martin Sebor  wrote:

> This locks a different mutex, one unrelated to __rw_facet::_C_mutex.

Let's debug this the good-ol' fashioned way: with fprintf(3C).  Let's
print the addresses of the mutex being locked in various places, and
see what we get:

1. include/loc/_numpunct.h:

template 
inline string numpunct<_CharT>::grouping () const
{
if (!(_C_flags & _RW::__rw_gr)) {

numpunct* const __self = _RWSTD_CONST_CAST (numpunct*, this);

// ###
(void) fprintf(stderr,
"%s: _RWSTD_MT_GUARD (__self->_C_mutex): 0x%x\n",
__func__, (void *) &(__self->_C_mutex));
// ###

_RWSTD_MT_GUARD (__self->_C_mutex);

if (!(_C_flags & _RW::__rw_gr)) {

// [try to] get the grouping first (may throw)
// then set a flag to avoid future initializations
__self->_C_grouping  = do_grouping ();
__self->_C_flags|= _RW::__rw_gr;

}
}

return _C_grouping;
}

2. src/facet.cpp:

const void* __rw_facet::_C_get_data ()
{
// check initialization
if (_C_impsize)
return _C_impdata;

// `pid' may be 0 if the facet has not been obtained
// by a call to use_facet (but instead constructed
// and used directly without having been installed
// in a locale object); in that case, locale database
// mapping for the facet is not available
if (!_C_pid)
return _C_impdata;

(void) fprintf(stderr, "%s: _RWSTD_MT_GUARD (&_C_mutex): 0x%x\n",
__func__, (void *) &_C_mutex);

// lock the object
_RWSTD_MT_GUARD (&_C_mutex);

[ ... ]

And now let's run it:

[steleman@darthvader][/src/steleman/programming/stdcxx-ss122/stdcxx-4.2.1/build/tests][09/06/2012
14:40:11][1084]>> ./22.locale.numpunct.mt --nthreads=2 --nloops=100
# INFO (S1) (10 lines):
# TEXT:
# COMPILER: SunPro, __SUNPRO_CC = 0x5120
# ENVIRONMENT: i386 running linux (Fedora release 17 (Beefy Miracle)
(3.5.0-2.fc17.x86_64)) with glibc 2.15
# FILE: 22.locale.numpunct.mt.cpp
# COMPILED: Sep  6 2012, 14:38:42
# COMMENT: thread safety


# CLAUSE: lib.locale.numpunct

# NOTE (S2) (5 lines):
# TEXT: executing "/usr/bin/locale -a > /tmp/tmpfile-YzXcb9"
# CLAUSE: lib.locale.numpunct
# FILE: process.cpp
# LINE: 276

grouping: _RWSTD_MT_GUARD (__self->_C_mutex): 0xf774913c
_C_get_data: _RWSTD_MT_GUARD (&_C_mutex): 0xf774913c

[ ... deadlock ... ]

Looks like the same mutex to me ...

--Stefan



-- 
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com

Re: A question (or two) of procedure, etc.


On 09/06/12 14:37, Wojciech Meyer wrote:

Liviu Nicoara  writes:


What is the latest policy in what regards trivial fixes, e.g., the
volatile qualifier for the max var in LIMITS.cpp we discussed earlier,
etc.? It seems excessive to create a bug report for such issues.


[...]
So I vote for keeping the changes as lightweight as possible, and avoid
extra bureaucracy if it makes sense. This assumption is based that
developers here trust their selves, and run the tests often. I'm not
subscribed to the commit list here, but if I do - I usually follow
people's changes and assume that developers do read commit lists.


Makes sense. Thanks.



So the general consensus from my experience with other project was: not
sure - ask.

That's my experience, also I don't have full rights to express my
opinion right now about stdcxx.


I sure hope we can have totally open (civilized) discussions going forward. :)




Also, IIUC from reading previous discussions, forward and backward
binary compatible changes go in 4.2.x, followed by merges to 4.3.x and
trunk. Am I getting this right?


Every project has certain branch strategy, I'm not sure about this so
maybe Martin can advice. I prefer to develop on trunk and cherry pick
to the other branches avoiding bulk merges (and that's in both
directions).


Right... I saw some discussions from a while back about active development on 4.2.x 
with integration to other branches as dictated by compatibility (e.g., integrating 
4.2.x -> 4.3.x and 4.2.x -> 4.2.1), and reintegration to trunk as needed. I am 
not looking to change any such policy just wanna make sure I am not messing something 
up.





Also, besides the Linux, FreeBSD, Windows, Solaris builds hosted on Apache 
(Jenkins) is anybody building on HP-UX, AIX, etc.?


Thanks.

Liviu

Re: New committers?

2012-09-06 Thread Jim Jagielski

If Christopher is interested in moving, then, to be frank, then
I expect him to do whatever work is required to move it, including
any legal legwork. This is esp true since his whole reason
for moving it is, as I mentioned, completely bogus.

My concern is to try to make it a success here.

On Sep 6, 2012, at 12:26 PM, Martin Sebor  wrote:

> On 09/06/2012 07:22 AM, Jim Jagielski wrote:
>> As an ASF project? It's not going to happen.
> 
> Not necessarily as an ASF project. Christopher is interested
> in moving the project somewhere else. See for example:
> 
> http://mail-archives.apache.org/mod_mbox/stdcxx-dev/201106.mbox/%3c4dfae25e.9040...@pathscale.com%3E
> 
> I would also like to know what the options are.
> 
> Martin
> 
>> 
>> On Sep 4, 2012, at 12:04 PM, Martin Sebor  wrote:
>> 
>>> On 09/02/2012 08:42 AM, Jim Jagielski wrote:
 
 On Sep 2, 2012, at 12:02 AM, Martin Sebor   wrote:
 
> On 08/31/2012 02:38 PM, Liviu Nicoara wrote:
>> My input below.
>> 
>> On 08/31/12 09:42, Wojciech Meyer wrote:
>>> The two significant ones (as far as I can understand):
>>> 
>>> - as I heard from Christopher Bergström that it's hard to push the
>>>   stdcxx to FreeBSD ports repository (I can understand it and that
>>>   sounds pretty bad, if that's the case then the board should consider
>>>   re-licensing as advised; I agree in general it's a hard decision for
>>>   the board, but imagine the project would benefit, IANAL tho)
>> 
>> Christopher's wishes and goals may be different from others'. I do not
>> believe he has ulterior motives that would be detrimental to the rest of
>> us but AFAICT he has not made a compelling argument. Even with one, it
>> stretches the imagination what could possibly convince Apache to give up
>> on STDCXX ownership.
> 
> Just a point of clarity: the ASF doesn't "own" stdcxx. They license
> it from Rogue Wave which still has the copyright. (Not that anyone
> there realizes it or would know what to do with it if they did.)
> IIUC, that's also why they can't relicense it under different terms.
> 
 
 FWIW, the ASF never requires copyright assignment... Just a copyright
 license to "reproduce, prepare derivative works of, publicly display,
 publicly perform, sublicense, and distribute Your Contributions and
 such derivative works."
 
 Also, there is nothing in our bylaws or in the various license
 agreements that *exclude* the ASF ever releasing code not under
 the ALv2 (how could it? After all, that would prevent us from
 ever being able to move to ALv3). Again, we could, if we wanted
 to (which we never will, btw) actually make our code under the
 GPLv2...
>>> 
>>> So what would it take to change the license to BSD as Christopher
>>> asks (IIUC)?
>>> 
>>> Martin
>>> 
>> 
>

Re: A question (or two) of procedure, etc.

2012-09-06 Thread Jim Jagielski

Trivial fixes should just be fixed... the normal expectation is
that bug reports are for non-trivial bugs or for trivial (and
non-trivial) bugs reported from the outside.

If a committers sees a bug, just go ahead and fix it, and
document the fix in a commit log, changefile, etc ;)

On Sep 6, 2012, at 2:06 PM, Liviu Nicoara  wrote:

> What is the latest policy in what regards trivial fixes, e.g., the volatile 
> qualifier for the max var in LIMITS.cpp we discussed earlier, etc.? It seems 
> excessive to create a bug report for such issues.
> 
> Also, IIUC from reading previous discussions, forward and backward binary 
> compatible changes go in 4.2.x, followed by merges to 4.3.x and trunk. Am I 
> getting this right?
> 
> Also, besides the Linux, FreeBSD, Windows, Solaris builds hosted on Apache 
> (Jenkins) is anybody building on HP-UX, AIX, etc.?
> 
> Thanks.
> 
> Liviu
>

Re: STDCXX-1056 [was: Re: STDCXX forks]

On Thu, Sep 6, 2012 at 2:46 PM, Stefan Teleman  wrote:

> [steleman@darthvader][/src/steleman/programming/stdcxx-ss122/stdcxx-4.2.1/build/tests][09/06/2012
> 14:40:11][1084]>> ./22.locale.numpunct.mt --nthreads=2 --nloops=100
> # INFO (S1) (10 lines):
> # TEXT:
> # COMPILER: SunPro, __SUNPRO_CC = 0x5120
> # ENVIRONMENT: i386 running linux (Fedora release 17 (Beefy Miracle)
> (3.5.0-2.fc17.x86_64)) with glibc 2.15
> # FILE: 22.locale.numpunct.mt.cpp
> # COMPILED: Sep  6 2012, 14:38:42
> # COMMENT: thread safety
> 
>
> # CLAUSE: lib.locale.numpunct
>
> # NOTE (S2) (5 lines):
> # TEXT: executing "/usr/bin/locale -a > /tmp/tmpfile-YzXcb9"
> # CLAUSE: lib.locale.numpunct
> # FILE: process.cpp
> # LINE: 276
>
> grouping: _RWSTD_MT_GUARD (__self->_C_mutex): 0xf774913c
> _C_get_data: _RWSTD_MT_GUARD (&_C_mutex): 0xf774913c
>
> [ ... deadlock ... ]

And just to make absolutely sure that this isn't a case of SunPro
being insane, here's the output from the Intel compiler:

[steleman@darthvader][/src/steleman/programming/stdcxx-intel/stdcxx-4.2.1-thread-safe/build/tests][09/06/2012
15:44:23][1390]>> ./22.locale.numpunct.mt --nthreads=2 --nloops=100
# INFO (S1) (10 lines):
# TEXT:
# COMPILER: Intel C++, __INTEL_COMPILER = 1210,
__INTEL_COMPILER_BUILD_DATE = 20111011, __EDG_VERSION__ = 403
# ENVIRONMENT: pentiumpro running linux-elf (Fedora release 17 (Beefy
Miracle) (3.5.0-2.fc17.x86_64)) with glibc 2.15
# FILE: 22.locale.numpunct.mt.cpp
# COMPILED: Sep  6 2012, 15:43:29
# COMMENT: thread safety


# CLAUSE: lib.locale.numpunct

# NOTE (S2) (5 lines):
# TEXT: executing "locale -a > /tmp/tmpfile-FoZR0J"
# CLAUSE: lib.locale.numpunct
# FILE: process.cpp
# LINE: 276

grouping: _RWSTD_MT_GUARD (__self->_C_mutex): 0xf74c2424
_C_get_data: _RWSTD_MT_GUARD (&_C_mutex): 0xf74c2424

[ ... deadlock ... ]

--Stefan

-- 
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com

Re: STDCXX-1056 [was: Re: STDCXX forks]


On 09/06/2012 12:46 PM, Stefan Teleman wrote:

On Thu, Sep 6, 2012 at 12:16 PM, Martin Sebor  wrote:


This locks a different mutex, one unrelated to __rw_facet::_C_mutex.


Let's debug this the good-ol' fashioned way: with fprintf(3C).  Let's
print the addresses of the mutex being locked in various places, and
see what we get:


...

grouping: _RWSTD_MT_GUARD (__self->_C_mutex): 0xf774913c
_C_get_data: _RWSTD_MT_GUARD (&_C_mutex): 0xf774913c

[ ... deadlock ... ]

Looks like the same mutex to me ...


These two are the same. The one you pointed to before
(__rw_facet::_C_manage at line 366) is a different one.

Now that we know what's causing the deadlock (and that
we can't use the same mutex in the inline function),
let's see if we can fix it in __rw_get_numpunct()
(and __rw_get_moneypunct()).

Here's a thought: it's not pretty but how about having
the function initialize the facet? It already has a pointer
to the base class, so it could downcast it to std::numpunct
(or moneypunct, respectively), and assign the initial values
to the members. Would that work?

Martin

Re: STDCXX-1056 [was: Re: STDCXX forks]

On Thu, Sep 6, 2012 at 4:02 PM, Martin Sebor  wrote:

> Here's a thought: it's not pretty but how about having
> the function initialize the facet? It already has a pointer
> to the base class, so it could downcast it to std::numpunct
> (or moneypunct, respectively), and assign the initial values
> to the members. Would that work?

I haven't looked at them in detail (yet) but a cursory look shows that
they're both recursive for the successful case.

--Stefan

-- 
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com

Re: STDCXX-1056 [was: Re: STDCXX forks]


On Sep 5, 2012, at 4:03 PM, Martin Sebor wrote:

> On 09/05/2012 01:33 PM, Liviu Nicoara wrote:
>> On 09/05/12 15:17, Martin Sebor wrote:
>>> On 09/05/2012 12:40 PM, Liviu Nicoara wrote:
 On 09/05/12 14:09, Stefan Teleman wrote:
> On Wed, Sep 5, 2012 at 10:52 AM, Martin Sebor  wrote:
> [...]
> OK so I did a little bit of testing, after looking at the *right*
> __rw_guard class. :-)
> 
> I changed the std::numpunct class thusly:
> [...]
 
 I am afraid this would be unsafe, too (if I said otherwise earlier I was
 wrong). [...] Thoughts?
>>> 
>>> You're right, there's still a problem. We didn't get the double
>>> checked locking quite right.
>> 
>> I wish for simplicity: eliminate the lazy initialization, and fully
>> initialize the facet in the constructor. Then we'd need no locking on
>> copying its data (std::string takes care of its own copying).
> 
> I'm not sure how easily we can do that. Almost all of locale
> is initialized lazily. Some of the layers might depend on the
> facets being initialized lazily as well. This was a deliberate
> design choice. One of the constraints was to avoid dynamic
> initialization or allocation at startup. [...]

There would be a performance degradation. IMHO, it would be minor and would 
simplify the code considerably.

After finally being able to reproduce the defect with SunPro 12.3 on x86_64 I 
tried to remove the lazy initialization and a (smaller) test case now passes. I 
am attaching the program and the numpunct patch. 

Will continue to look into it. Although STDCXX does not have an implementation 
of the atomics library we could probably use the existing, unexposed, atomics 
API to implement a more robust lazy initialization. 

Liviu

$ cat tests/localization/t.cpp; svn diff
#include 

#include 
#include 

#include 

#define MAX_THREADS16
#define MAX_LOOPS1000

static bool hold = true;

extern "C" {

static void* 
f (void* pv)
{
std::numpunct const& fac = 
*reinterpret_cast< std::numpunct* > (pv);

while (hold) ; 

for (int i = 0; i != MAX_LOOPS; ++i) {
std::string const grp = fac.grouping ();
}

return 0;
}

}

int
main (int argc, char** argv)
{
std::locale const loc = std::locale (argv [1]);

std::numpunct const& fac =
std::use_facet > (loc);

pthread_t tid [MAX_THREADS] = { 0 };

for (int i = 0; i < MAX_THREADS; ++i) {
if (pthread_create (tid + i, 0, f, (void*)&fac)) 
exit (-1);
}

sleep (1);

hold = false;

for (int i = 0; i < MAX_THREADS; ++i) {
if (tid [i])
pthread_join (tid [i], 0);
}

return 0;
}

Index: include/loc/_numpunct.h
===
--- include/loc/_numpunct.h (revision 1381575)
+++ include/loc/_numpunct.h (working copy)
@@ -61,24 +61,40 @@ struct numpunct: _RW::__rw_facet
 string_type;
 
 _EXPLICIT numpunct (_RWSTD_SIZE_T __ref = 0)
-: _RW::__rw_facet (__ref), _C_flags (0) { }
+: _RW::__rw_facet (__ref) {
+_C_grouping   = do_grouping ();
+_C_truename   = do_truename ();
+_C_falsename  = do_falsename ();
+_C_decimal_point  = do_decimal_point ();
+_C_thousands_sep  = do_thousands_sep ();
+}
 
 virtual ~numpunct () _RWSTD_ATTRIBUTE_NOTHROW;
 
 // 22.2.3.1.1, p1
-char_type decimal_point () const;
+char_type decimal_point () const {
+return _C_decimal_point;
+}
 
 // 22.2.3.1.1, p2
-char_type thousands_sep () const;
+char_type thousands_sep () const {
+return _C_thousands_sep;
+}
 
 // 22.2.3.1.1, p3
-string grouping () const;
+string grouping () const {
+return _C_grouping;
+}
 
 // 22.2.3.1.1, p4
-string_type truename () const;
+string_type truename () const {
+return _C_truename;
+}
 
 // 22.2.3.1.1, p4
-string_type falsename () const;
+string_type falsename () const {
+return _C_falsename;
+}
 
 static _RW::__rw_facet_id id;
 
@@ -112,15 +128,13 @@ protected:
 
 private:
 
-int _C_flags;   // bitmap of "cached data valid" flags
-string  _C_grouping;// cached results of virtual members
+string  _C_grouping;
 string_type _C_truename;
 string_type _C_falsename;
 char_type   _C_decimal_point;
 char_type   _C_thousands_sep;
 };
 
-
 #ifndef _RWSTD_NO_SPECIALIZED_FACET_ID
 
 _RWSTD_SPECIALIZED_CLASS
@@ -134,95 +148,6 @@ _RW::__rw_facet_id numpunct::id
 #  endif   // _RWSTD_NO_WCHAR_T
 #endif   // _RWSTD_NO_SPECIALIZED_FACET_ID
 
-
-template 
-inline _TYPENAME numpunct<_CharT>::char_type
-numpunct<_CharT>::decimal_point () const
-{
-if (!(_C_flags & _RW::__rw_dp)) {
-
-numpunct* const __self = _RWSTD_CONST_CAST (numpunct*, this);
-
-// [try to] get the decimal point first (may throw)
-// then set a flag to avoid future initi

Re: STDCXX-1056 [was: Re: STDCXX forks]


I'm not sure how easily we can do that. Almost all of locale
is initialized lazily. Some of the layers might depend on the
facets being initialized lazily as well. This was a deliberate
design choice. One of the constraints was to avoid dynamic
initialization or allocation at startup. [...]


There would be a performance degradation. IMHO, it would be minor and would 
simplify the code considerably.

After finally being able to reproduce the defect with SunPro 12.3 on x86_64 I 
tried to remove the lazy initialization and a (smaller) test case now passes. I 
am attaching the program and the numpunct patch.


Out of curiosity, does it still work if you move the locale into
the thread function? Like this:


static void*
f (void* pv)
{
 std::numpunct  const&  fac =
 *reinterpret_cast<  std::numpunct*>  (pv);

 while (hold) ;

 for (int i = 0; i != MAX_LOOPS; ++i) {


   const std::locale loc ("en_US.utf8");

   typedef std::numpunct NumPunct;
   NumPunct const& fac = std::use_facet(loc);


 std::string const grp = fac.grouping ();
 }

 return 0;
}



Also, does the 27.objects test pass with this patch?

I don't know if we have tests for it but writing to cerr/cout
in a bad_alloc handler should succeed. I.e., this should not
abort:

  try {
  struct rlimit rlim = { };
  getrlimit (RLIMIT_DATA, &rlim);
  rlim.rlim_cur = 0;
  setrlimit (RLIMIT_DATA, &rlim);

  for ( ; ; ) new int;
  }
  catch (bad_alloc) {
  cout << "caught bad alloc: << 123 << '\n';
  }

Martin

Re: A question (or two) of procedure, etc.


Anyone is welcome to express their opinion here, especially
if you are or have in the past contributed to the project.
The weight of the opinion is (or should be) commensurate to
the value of the contributions. I think the ASF calls this
Meritocracy.

I made the stdcxx process increasingly more formal as I learned
from my own past mistakes that a loose process makes it harder
to track changes and find the root cause of the problems they
sometimes introduce. In practical terms, I've made an effort
to have an issue, with a test case if possible, for every
change made to the code, and commit a regression test into
the test suite for every bug fix.

FWIW, in my day to day job, this is a requirement. Cisco
doesn't make a change to its code without an issue. My team
does the same with GCC changes. We find that projects that
don't follow this practice as closely (e.g., GNU Binutils),
tend to be more difficult for us to work with than those
that do.

That being said, when it comes to the stdcxx configuration
machinery, or to the test suite, I think it's fine to be
somewhat less formal. We don't need test cases for problems
in configuration tests, or necessarily even test cases
reproducing failures in library tests (although small tests
can often be more useful than the large tests we have in
the test suite). We also don't need tests for makefile bugs.
Outside of that, when it comes to changing the library, I
do recommend making an effort to create test cases and open
issues for all changes.

Martin

On 09/06/2012 12:37 PM, Wojciech Meyer wrote:

Liviu Nicoara  writes:


What is the latest policy in what regards trivial fixes, e.g., the
volatile qualifier for the max var in LIMITS.cpp we discussed earlier,
etc.? It seems excessive to create a bug report for such issues.


My advice based on some observations with other projects, is that in
these cases we go just go on and apply fix. Non invasive code quality
improvements over the codebase should be promoted not hindered. More
risky patches, should be discussed on the list, the ones that needs
either serious changes, attention, re-factoring, feature or fixes that
may break something should be logged in Jira.

So I vote for keeping the changes as lightweight as possible, and avoid
extra bureaucracy if it makes sense. This assumption is based that
developers here trust their selves, and run the tests often. I'm not
subscribed to the commit list here, but if I do - I usually follow
people's changes and assume that developers do read commit lists.

So the general consensus from my experience with other project was: not
sure - ask.

That's my experience, also I don't have full rights to express my
opinion right now about stdcxx.


Also, IIUC from reading previous discussions, forward and backward
binary compatible changes go in 4.2.x, followed by merges to 4.3.x and
trunk. Am I getting this right?


Every project has certain branch strategy, I'm not sure about this so
maybe Martin can advice. I prefer to develop on trunk and cherry pick
to the other branches avoiding bulk merges (and that's in both
directions).



Also, besides the Linux, FreeBSD, Windows, Solaris builds hosted on Apache 
(Jenkins) is anybody building on HP-UX, AIX, etc.?

Thanks.

Liviu



--
Wojciech Meyer
http://danmey.org

Re: A question (or two) of procedure, etc.


One thing I forgot to mention: we have three active branches,
and, for better or worse, most changes tend to get committed
to 4.2.x first. It's easy to forget or delay committing the
same change to 4.3.x and trunk. Having an issue in Jira
serves as a reminder to also commit the change to the other
branches. (At least until we start doing development on
trunk.)

On 09/06/2012 08:36 PM, Martin Sebor wrote:

Anyone is welcome to express their opinion here, especially
if you are or have in the past contributed to the project.
The weight of the opinion is (or should be) commensurate to
the value of the contributions. I think the ASF calls this
Meritocracy.

I made the stdcxx process increasingly more formal as I learned
from my own past mistakes that a loose process makes it harder
to track changes and find the root cause of the problems they
sometimes introduce. In practical terms, I've made an effort
to have an issue, with a test case if possible, for every
change made to the code, and commit a regression test into
the test suite for every bug fix.

FWIW, in my day to day job, this is a requirement. Cisco
doesn't make a change to its code without an issue. My team
does the same with GCC changes. We find that projects that
don't follow this practice as closely (e.g., GNU Binutils),
tend to be more difficult for us to work with than those
that do.

That being said, when it comes to the stdcxx configuration
machinery, or to the test suite, I think it's fine to be
somewhat less formal. We don't need test cases for problems
in configuration tests, or necessarily even test cases
reproducing failures in library tests (although small tests
can often be more useful than the large tests we have in
the test suite). We also don't need tests for makefile bugs.
Outside of that, when it comes to changing the library, I
do recommend making an effort to create test cases and open
issues for all changes.

Martin

On 09/06/2012 12:37 PM, Wojciech Meyer wrote:

Liviu Nicoara  writes:


What is the latest policy in what regards trivial fixes, e.g., the
volatile qualifier for the max var in LIMITS.cpp we discussed earlier,
etc.? It seems excessive to create a bug report for such issues.


My advice based on some observations with other projects, is that in
these cases we go just go on and apply fix. Non invasive code quality
improvements over the codebase should be promoted not hindered. More
risky patches, should be discussed on the list, the ones that needs
either serious changes, attention, re-factoring, feature or fixes that
may break something should be logged in Jira.

So I vote for keeping the changes as lightweight as possible, and avoid
extra bureaucracy if it makes sense. This assumption is based that
developers here trust their selves, and run the tests often. I'm not
subscribed to the commit list here, but if I do - I usually follow
people's changes and assume that developers do read commit lists.

So the general consensus from my experience with other project was: not
sure - ask.

That's my experience, also I don't have full rights to express my
opinion right now about stdcxx.


Also, IIUC from reading previous discussions, forward and backward
binary compatible changes go in 4.2.x, followed by merges to 4.3.x and
trunk. Am I getting this right?


Every project has certain branch strategy, I'm not sure about this so
maybe Martin can advice. I prefer to develop on trunk and cherry pick
to the other branches avoiding bulk merges (and that's in both
directions).



Also, besides the Linux, FreeBSD, Windows, Solaris builds hosted on
Apache (Jenkins) is anybody building on HP-UX, AIX, etc.?

Thanks.

Liviu



--
Wojciech Meyer
http://danmey.org

Re: STDCXX-1056 [was: Re: STDCXX forks]

On Thu, Sep 6, 2012 at 7:31 PM, Liviu Nicoara  wrote:

> There would be a performance degradation. IMHO, it would be minor and would 
> simplify the code considerably.
>
> After finally being able to reproduce the defect with SunPro 12.3 on x86_64 I 
> tried to remove the lazy initialization and a (smaller) test case now passes. 
> I am attaching the program and the numpunct patch.

With your patches, the performance is much much better:

# INFO (S1) (10 lines):
# TEXT:
# COMPILER: Intel C++, __INTEL_COMPILER = 1210,
__INTEL_COMPILER_BUILD_DATE = 20111011, __EDG_VERSION__ = 403
# ENVIRONMENT: pentiumpro running linux-elf (Fedora release 17 (Beefy
Miracle) (3.5.0-2.fc17.x86_64)) with glibc 2.15
# FILE: 22.locale.numpunct.mt.cpp
# COMPILED: Sep  6 2012, 20:50:13
# COMMENT: thread safety


# +---+--+--+--+
# | DIAGNOSTIC|  ACTIVE  |   TOTAL  | INACTIVE |
# +---+--+--+--+
# | (S1) INFO |   11 |   11 |   0% |
# | (S2) NOTE |1 |1 |   0% |
# | (S8) ERROR|0 |3 | 100% |
# | (S9) FATAL|0 |1 | 100% |
# +---+--+--+--+
real 1035.05
user 1191.76
sys 63.49

--Stefan

-- 
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com

Re: A question (or two) of procedure, etc.