All of lore.kernel.org
 help / color / mirror / Atom feed
* C++11, std::list::size(), and trusty
@ 2017-11-10 17:47 Sage Weil
  2017-11-13  9:29 ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: Sage Weil @ 2017-11-10 17:47 UTC (permalink / raw)
  To: ceph-devel, ceph-maintainers; +Cc: kchai, ncutler, james.page

We noticed a big performance regression when switching some code to 
use list::size() because although C++11 promizes that it is O(1), some of 
the libstdc++'s out there are still O(n).  This PR aims to fix that

	https://github.com/ceph/ceph/pull/18863

by adding the various devtoolset packages as dependencies to pull in 
updated build toolchains.  For el7 that's devtoolset-7-{binutils,gcc-c++}.

I'm not sure what the requirement for deb-based distros is.  
ubuntu-toolchain-r?

My preference would to move our build requirements forward so that you 
have to have a modern libstdc++ in order to build, allowing us to make 
C++11-ish assumptions (like an O(1) list::size()).

1) Is this needed for xenial, or just trusty?

2) If only trusty, should we do something weird here to keep 
buliding for it post-luminous, or can we (finally!) drop the trusty 
builds?

sage

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-10 17:47 C++11, std::list::size(), and trusty Sage Weil
@ 2017-11-13  9:29 ` kefu chai
  2017-11-14 12:49   ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: kefu chai @ 2017-11-13  9:29 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel, ceph-maintainers, kchai, Nathan Cutler, james.page

On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
> We noticed a big performance regression when switching some code to
> use list::size() because although C++11 promizes that it is O(1), some of
> the libstdc++'s out there are still O(n).  This PR aims to fix that
>
>         https://github.com/ceph/ceph/pull/18863
>
> by adding the various devtoolset packages as dependencies to pull in
> updated build toolchains.  For el7 that's devtoolset-7-{binutils,gcc-c++}.
>
> I'm not sure what the requirement for deb-based distros is.
> ubuntu-toolchain-r?

for ubuntu trusty, yes. for old stable (jessie) of debian, not sure if
"apt-get -t experimental" works or not.

>
> My preference would to move our build requirements forward so that you
> have to have a modern libstdc++ in order to build, allowing us to make
> C++11-ish assumptions (like an O(1) list::size()).
>
> 1) Is this needed for xenial, or just trusty?

it's just trusty. xenial comes with gcc 5.3.1, see
https://packages.ubuntu.com/search?keywords=gcc&searchon=names&suite=xenial&section=all.

>
> 2) If only trusty, should we do something weird here to keep
> buliding for it post-luminous, or can we (finally!) drop the trusty
> builds?

i'd vote for dropping it. but if we need to stick with trusty, here
comes the weird thing: https://github.com/ceph/ceph-build/pull/914 =)

>
> sage
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-13  9:29 ` kefu chai
@ 2017-11-14 12:49   ` kefu chai
       [not found]     ` <CACJqLyamZeEro6htJDV3EDohtH=WbQaHFFXkiRLWiC3t7hWHrA@mail.gmail.com>
  2017-11-14 22:04     ` Ken Dreyer
  0 siblings, 2 replies; 30+ messages in thread
From: kefu chai @ 2017-11-14 12:49 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel, ceph-maintainers, kchai, Nathan Cutler, James Page

On Mon, Nov 13, 2017 at 5:29 PM, kefu chai <tchaikov@gmail.com> wrote:
> On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
>> We noticed a big performance regression when switching some code to
>> use list::size() because although C++11 promizes that it is O(1), some of
>> the libstdc++'s out there are still O(n).  This PR aims to fix that
>>
>>         https://github.com/ceph/ceph/pull/18863
>>
>> by adding the various devtoolset packages as dependencies to pull in
>> updated build toolchains.  For el7 that's devtoolset-7-{binutils,gcc-c++}.
>>

when running the binaries built using GCC 5.1 in an env where an old
libstdc++ (typically comes with GCC 4.8):

$ rados
rados: relocation error: /usr/lib/ceph/libceph-common.so.0: symbol
_ZTINSt8ios_base7failureB5cxx11E, version GLIBCXX_3.4.21 not defined
in file libstdc++.so.6 with link time reference

because libstdc++ introduced a new ABI which is incompatible with the old one.
see https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
and https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ .

in other words, we need to either 1) link statically with libstdc++.so or
2) include it in librados2 on distros with GCC version less than 5.1. because
ceph-osd (and other daemons packages) => ceph-base => ceph-common =>
python-rados => librados2.

i think the 2) approach is better. what do you think?


>> I'm not sure what the requirement for deb-based distros is.
>> ubuntu-toolchain-r?
>
> for ubuntu trusty, yes. for old stable (jessie) of debian, not sure if
> "apt-get -t experimental" works or not.
>
>>
>> My preference would to move our build requirements forward so that you
>> have to have a modern libstdc++ in order to build, allowing us to make
>> C++11-ish assumptions (like an O(1) list::size()).
>>
>> 1) Is this needed for xenial, or just trusty?
>
> it's just trusty. xenial comes with gcc 5.3.1, see
> https://packages.ubuntu.com/search?keywords=gcc&searchon=names&suite=xenial&section=all.
>
>>
>> 2) If only trusty, should we do something weird here to keep
>> buliding for it post-luminous, or can we (finally!) drop the trusty
>> builds?
>
> i'd vote for dropping it. but if we need to stick with trusty, here
> comes the weird thing: https://github.com/ceph/ceph-build/pull/914 =)
>
>>
>> sage
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Regards
> Kefu Chai



-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
       [not found]       ` <CACJqLyZ=Q4rNHZ9y8-DEUdSqCAK8UyjVg9QXYFzxqTf_Rvf-hg@mail.gmail.com>
@ 2017-11-14 13:06         ` kefu chai
  0 siblings, 0 replies; 30+ messages in thread
From: kefu chai @ 2017-11-14 13:06 UTC (permalink / raw)
  To: Haomai Wang
  Cc: James Page, Nathan Cutler, Sage Weil, ceph-devel,
	ceph-maintainers, kchai

On Tue, Nov 14, 2017 at 8:58 PM, Haomai Wang <haomai@xsky.com> wrote:
>
> Haomai Wang <haomai@xsky.com>于2017年11月14日 周二20:57写道:
>>
>>
>> kefu chai <tchaikov@gmail.com>于2017年11月14日 周二20:51写道:
>>>
>>> On Mon, Nov 13, 2017 at 5:29 PM, kefu chai <tchaikov@gmail.com> wrote:
>>> > On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
>>> >> We noticed a big performance regression when switching some code to
>>> >> use list::size() because although C++11 promizes that it is O(1), some
>>> >> of
>>> >> the libstdc++'s out there are still O(n).  This PR aims to fix that
>>> >>
>>> >>         https://github.com/ceph/ceph/pull/18863
>>> >>
>>> >> by adding the various devtoolset packages as dependencies to pull in
>>> >> updated build toolchains.  For el7 that's
>>> >> devtoolset-7-{binutils,gcc-c++}.
>>> >>
>>>
>>> when running the binaries built using GCC 5.1 in an env where an old
>>> libstdc++ (typically comes with GCC 4.8):
>>>
>>> $ rados
>>> rados: relocation error: /usr/lib/ceph/libceph-common.so.0: symbol
>>> _ZTINSt8ios_base7failureB5cxx11E, version GLIBCXX_3.4.21 not defined
>>> in file libstdc++.so.6 with link time reference
>>>
>>> because libstdc++ introduced a new ABI which is incompatible with the old
>>> one.
>>> see https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
>>> and https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ .
>>>
>>> in other words, we need to either 1) link statically with libstdc++.so or
>>> 2) include it in librados2 on distros with GCC version less than 5.1.
>>> because
>>> ceph-osd (and other daemons packages) => ceph-base => ceph-common =>
>>> python-rados => librados2.
>>>
>>> i think the 2) approach is better. what do you think?
>>
>>
>> Do we consider to keep it? I checked the usage of std::c++
>
> list
>
>> , most is ok. Large list only used by pglog which is newly introdeced
>>

i guess by "keep it", what you mean is to keep using whatever the
toolchain comes with the distro.

yeah, we *can* live with it. but a standard conforming C++
implementation can save us more time
in long term. it took me at least 1 whole day in total to understand
that 1) the std::regex in GCC 4.8
is incomplete and buggy 2) list::size() in GCC 4.8 is O(n).

the list will be growing over the time. i am sure.

>>>
>>>
>>> >> I'm not sure what the requirement for deb-based distros is.
>>> >> ubuntu-toolchain-r?
>>> >
>>> > for ubuntu trusty, yes. for old stable (jessie) of debian, not sure if
>>> > "apt-get -t experimental" works or not.
>>> >
>>> >>
>>> >> My preference would to move our build requirements forward so that you
>>> >> have to have a modern libstdc++ in order to build, allowing us to make
>>> >> C++11-ish assumptions (like an O(1) list::size()).
>>> >>
>>> >> 1) Is this needed for xenial, or just trusty?
>>> >
>>> > it's just trusty. xenial comes with gcc 5.3.1, see
>>> >
>>> > https://packages.ubuntu.com/search?keywords=gcc&searchon=names&suite=xenial&section=all.
>>> >
>>> >>
>>> >> 2) If only trusty, should we do something weird here to keep
>>> >> buliding for it post-luminous, or can we (finally!) drop the trusty
>>> >> builds?
>>> >
>>> > i'd vote for dropping it. but if we need to stick with trusty, here
>>> > comes the weird thing: https://github.com/ceph/ceph-build/pull/914 =)
>>> >
>>> >>
>>> >> sage
>>> >> --
>>> >> To unsubscribe from this list: send the line "unsubscribe ceph-devel"
>>> >> in
>>> >> the body of a message to majordomo@vger.kernel.org
>>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> >
>>> >
>>> >
>>> > --
>>> > Regards
>>> > Kefu Chai
>>>
>>>
>>>
>>> --
>>> Regards
>>> Kefu Chai
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-14 12:49   ` kefu chai
       [not found]     ` <CACJqLyamZeEro6htJDV3EDohtH=WbQaHFFXkiRLWiC3t7hWHrA@mail.gmail.com>
@ 2017-11-14 22:04     ` Ken Dreyer
  2017-11-15  6:47       ` kefu chai
  1 sibling, 1 reply; 30+ messages in thread
From: Ken Dreyer @ 2017-11-14 22:04 UTC (permalink / raw)
  To: kefu chai
  Cc: Sage Weil, ceph-devel, ceph-maintainers, kchai, Nathan Cutler,
	James Page

On Tue, Nov 14, 2017 at 5:49 AM, kefu chai <tchaikov@gmail.com> wrote:
> On Mon, Nov 13, 2017 at 5:29 PM, kefu chai <tchaikov@gmail.com> wrote:
>> On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
>>> We noticed a big performance regression when switching some code to
>>> use list::size() because although C++11 promizes that it is O(1), some of
>>> the libstdc++'s out there are still O(n).  This PR aims to fix that
>>>
>>>         https://github.com/ceph/ceph/pull/18863
>>>
>>> by adding the various devtoolset packages as dependencies to pull in
>>> updated build toolchains.  For el7 that's devtoolset-7-{binutils,gcc-c++}.
>>>
>
> when running the binaries built using GCC 5.1 in an env where an old
> libstdc++ (typically comes with GCC 4.8):
>
> $ rados
> rados: relocation error: /usr/lib/ceph/libceph-common.so.0: symbol
> _ZTINSt8ios_base7failureB5cxx11E, version GLIBCXX_3.4.21 not defined
> in file libstdc++.so.6 with link time reference
>
> because libstdc++ introduced a new ABI which is incompatible with the old one.
> see https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
> and https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ .
>
> in other words, we need to either 1) link statically with libstdc++.so or
> 2) include it in librados2 on distros with GCC version less than 5.1. because
> ceph-osd (and other daemons packages) => ceph-base => ceph-common =>
> python-rados => librados2.

For 1), what does that imply for packages' debuginfo sizes?

For 2), where would libstdc++.so ship in the filesystem? What does
that mean for other applications that would load libstdc++.so and also
link with ceph?

- Ken

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-14 22:04     ` Ken Dreyer
@ 2017-11-15  6:47       ` kefu chai
  2017-11-30 10:15         ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: kefu chai @ 2017-11-15  6:47 UTC (permalink / raw)
  To: Ken Dreyer
  Cc: Sage Weil, ceph-devel, ceph-maintainers, kchai, Nathan Cutler,
	James Page

On Wed, Nov 15, 2017 at 6:04 AM, Ken Dreyer <kdreyer@redhat.com> wrote:
> On Tue, Nov 14, 2017 at 5:49 AM, kefu chai <tchaikov@gmail.com> wrote:
>> On Mon, Nov 13, 2017 at 5:29 PM, kefu chai <tchaikov@gmail.com> wrote:
>>> On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
>>>> We noticed a big performance regression when switching some code to
>>>> use list::size() because although C++11 promizes that it is O(1), some of
>>>> the libstdc++'s out there are still O(n).  This PR aims to fix that
>>>>
>>>>         https://github.com/ceph/ceph/pull/18863
>>>>
>>>> by adding the various devtoolset packages as dependencies to pull in
>>>> updated build toolchains.  For el7 that's devtoolset-7-{binutils,gcc-c++}.
>>>>
>>
>> when running the binaries built using GCC 5.1 in an env where an old
>> libstdc++ (typically comes with GCC 4.8):
>>
>> $ rados
>> rados: relocation error: /usr/lib/ceph/libceph-common.so.0: symbol
>> _ZTINSt8ios_base7failureB5cxx11E, version GLIBCXX_3.4.21 not defined
>> in file libstdc++.so.6 with link time reference
>>
>> because libstdc++ introduced a new ABI which is incompatible with the old one.
>> see https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
>> and https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ .
>>
>> in other words, we need to either 1) link statically with libstdc++.so or
>> 2) include it in librados2 on distros with GCC version less than 5.1. because
>> ceph-osd (and other daemons packages) => ceph-base => ceph-common =>
>> python-rados => librados2.
>
> For 1), what does that imply for packages' debuginfo sizes?

since we don't compile the source of libstdc++, i don't think this
will increase the size of debuginfo.

>
> For 2), where would libstdc++.so ship in the filesystem? What does
> that mean for other applications that would load libstdc++.so and also
> link with ceph?

if we just statically link libceph-common against libstdc++, and let
daemons link againt libceph-common, the problem will be solved. that's
actually 2) but after a second thought, i think it's a better
approach. please note, the daemons are linked against libceph-common
statically. [OT] we could let daemon link libceph-common dynamically,
but the downside is that the daemon will need to depend on a client
side library: librados2. which is a little bit weird. but the upside
is: this will shrink the size of debug-info and the size of daemons,
as the daemon will not include ceph-common statically.




-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-15  6:47       ` kefu chai
@ 2017-11-30 10:15         ` kefu chai
  2017-11-30 11:00           ` Nathan Cutler
  0 siblings, 1 reply; 30+ messages in thread
From: kefu chai @ 2017-11-30 10:15 UTC (permalink / raw)
  To: Ken Dreyer
  Cc: Sage Weil, ceph-devel, ceph-maintainers, kchai, Nathan Cutler,
	James Page, Alfredo Deza

On Wed, Nov 15, 2017 at 2:47 PM, kefu chai <tchaikov@gmail.com> wrote:
> On Wed, Nov 15, 2017 at 6:04 AM, Ken Dreyer <kdreyer@redhat.com> wrote:
>> On Tue, Nov 14, 2017 at 5:49 AM, kefu chai <tchaikov@gmail.com> wrote:
>>> On Mon, Nov 13, 2017 at 5:29 PM, kefu chai <tchaikov@gmail.com> wrote:
>>>> On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
>>>>> We noticed a big performance regression when switching some code to
>>>>> use list::size() because although C++11 promizes that it is O(1), some of
>>>>> the libstdc++'s out there are still O(n).  This PR aims to fix that
>>>>>
>>>>>         https://github.com/ceph/ceph/pull/18863
>>>>>
>>>>> by adding the various devtoolset packages as dependencies to pull in
>>>>> updated build toolchains.  For el7 that's devtoolset-7-{binutils,gcc-c++}.
>>>>>
>>>
>>> when running the binaries built using GCC 5.1 in an env where an old
>>> libstdc++ (typically comes with GCC 4.8):
>>>
>>> $ rados
>>> rados: relocation error: /usr/lib/ceph/libceph-common.so.0: symbol
>>> _ZTINSt8ios_base7failureB5cxx11E, version GLIBCXX_3.4.21 not defined
>>> in file libstdc++.so.6 with link time reference
>>>
>>> because libstdc++ introduced a new ABI which is incompatible with the old one.
>>> see https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
>>> and https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ .
>>>
>>> in other words, we need to either 1) link statically with libstdc++.so or
>>> 2) include it in librados2 on distros with GCC version less than 5.1. because
>>> ceph-osd (and other daemons packages) => ceph-base => ceph-common =>
>>> python-rados => librados2.
>>
>> For 1), what does that imply for packages' debuginfo sizes?
>
> since we don't compile the source of libstdc++, i don't think this
> will increase the size of debuginfo.
>
>>
>> For 2), where would libstdc++.so ship in the filesystem? What does
>> that mean for other applications that would load libstdc++.so and also
>> link with ceph?
>
> if we just statically link libceph-common against libstdc++, and let
> daemons link againt libceph-common, the problem will be solved. that's
> actually 2) but after a second thought, i think it's a better
> approach. please note, the daemons are linked against libceph-common
> statically. [OT] we could let daemon link libceph-common dynamically,
> but the downside is that the daemon will need to depend on a client
> side library: librados2. which is a little bit weird. but the upside
> is: this will shrink the size of debug-info and the size of daemons,
> as the daemon will not include ceph-common statically.

i just confirmed with the devtoolset maintainer offline, that the bits
not included by
the base libstdc++ will be statically linked into the binary. and i
verified this behavior
on centos7. but this is not true, when it comes to trusty.


$ bin/rados
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`CXXABI_1.3.8' not found (required by bin/rados)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`CXXABI_1.3.8' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`GLIBCXX_3.4.21' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`GLIBCXX_3.4.22' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`CXXABI_1.3.8' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`GLIBCXX_3.4.21' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`GLIBCXX_3.4.22' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`CXXABI_1.3.8' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`GLIBCXX_3.4.22' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`GLIBCXX_3.4.21' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
`GLIBCXX_3.4.20' not found (required by
/var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)

so, an obvious solution is to use "-static-libstdc++"[1] on trusty,
libceph-common is a good candidate for where the libstdc++ is
included. or just drop the support of it. thoughts?

---
[1] https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html

-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-30 10:15         ` kefu chai
@ 2017-11-30 11:00           ` Nathan Cutler
  2017-11-30 14:38             ` Sage Weil
  0 siblings, 1 reply; 30+ messages in thread
From: Nathan Cutler @ 2017-11-30 11:00 UTC (permalink / raw)
  To: kefu chai, Ken Dreyer
  Cc: ceph-maintainers, Alfredo Deza, kchai, Sage Weil, James Page, ceph-devel

I thought we already agreed to drop Trusty in Mimic?

On 11/30/2017 11:15 AM, kefu chai wrote:
> On Wed, Nov 15, 2017 at 2:47 PM, kefu chai <tchaikov@gmail.com> wrote:
>> On Wed, Nov 15, 2017 at 6:04 AM, Ken Dreyer <kdreyer@redhat.com> wrote:
>>> On Tue, Nov 14, 2017 at 5:49 AM, kefu chai <tchaikov@gmail.com> wrote:
>>>> On Mon, Nov 13, 2017 at 5:29 PM, kefu chai <tchaikov@gmail.com> wrote:
>>>>> On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
>>>>>> We noticed a big performance regression when switching some code to
>>>>>> use list::size() because although C++11 promizes that it is O(1), some of
>>>>>> the libstdc++'s out there are still O(n).  This PR aims to fix that
>>>>>>
>>>>>>          https://github.com/ceph/ceph/pull/18863
>>>>>>
>>>>>> by adding the various devtoolset packages as dependencies to pull in
>>>>>> updated build toolchains.  For el7 that's devtoolset-7-{binutils,gcc-c++}.
>>>>>>
>>>>
>>>> when running the binaries built using GCC 5.1 in an env where an old
>>>> libstdc++ (typically comes with GCC 4.8):
>>>>
>>>> $ rados
>>>> rados: relocation error: /usr/lib/ceph/libceph-common.so.0: symbol
>>>> _ZTINSt8ios_base7failureB5cxx11E, version GLIBCXX_3.4.21 not defined
>>>> in file libstdc++.so.6 with link time reference
>>>>
>>>> because libstdc++ introduced a new ABI which is incompatible with the old one.
>>>> see https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
>>>> and https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ .
>>>>
>>>> in other words, we need to either 1) link statically with libstdc++.so or
>>>> 2) include it in librados2 on distros with GCC version less than 5.1. because
>>>> ceph-osd (and other daemons packages) => ceph-base => ceph-common =>
>>>> python-rados => librados2.
>>>
>>> For 1), what does that imply for packages' debuginfo sizes?
>>
>> since we don't compile the source of libstdc++, i don't think this
>> will increase the size of debuginfo.
>>
>>>
>>> For 2), where would libstdc++.so ship in the filesystem? What does
>>> that mean for other applications that would load libstdc++.so and also
>>> link with ceph?
>>
>> if we just statically link libceph-common against libstdc++, and let
>> daemons link againt libceph-common, the problem will be solved. that's
>> actually 2) but after a second thought, i think it's a better
>> approach. please note, the daemons are linked against libceph-common
>> statically. [OT] we could let daemon link libceph-common dynamically,
>> but the downside is that the daemon will need to depend on a client
>> side library: librados2. which is a little bit weird. but the upside
>> is: this will shrink the size of debug-info and the size of daemons,
>> as the daemon will not include ceph-common statically.
> 
> i just confirmed with the devtoolset maintainer offline, that the bits
> not included by
> the base libstdc++ will be statically linked into the binary. and i
> verified this behavior
> on centos7. but this is not true, when it comes to trusty.
> 
> 
> $ bin/rados
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `CXXABI_1.3.8' not found (required by bin/rados)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `CXXABI_1.3.8' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `GLIBCXX_3.4.21' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `GLIBCXX_3.4.22' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `CXXABI_1.3.8' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `GLIBCXX_3.4.21' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `GLIBCXX_3.4.22' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `CXXABI_1.3.8' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `GLIBCXX_3.4.22' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `GLIBCXX_3.4.21' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> `GLIBCXX_3.4.20' not found (required by
> /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> 
> so, an obvious solution is to use "-static-libstdc++"[1] on trusty,
> libceph-common is a good candidate for where the libstdc++ is
> included. or just drop the support of it. thoughts?
> 
> ---
> [1] https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html
> 

-- 
Nathan Cutler
Software Engineer Distributed Storage
SUSE LINUX, s.r.o.
Tel.: +420 284 084 037

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-30 11:00           ` Nathan Cutler
@ 2017-11-30 14:38             ` Sage Weil
  2017-12-05 14:53               ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: Sage Weil @ 2017-11-30 14:38 UTC (permalink / raw)
  To: Nathan Cutler
  Cc: kefu chai, Ken Dreyer, ceph-maintainers, Alfredo Deza, kchai,
	James Page, ceph-devel

On Thu, 30 Nov 2017, Nathan Cutler wrote:
> I thought we already agreed to drop Trusty in Mimic?

I'm not sure we made any final decision, but I haven't seen anyone 
complain this time around, so I'd say let's do it!

sage


> On 11/30/2017 11:15 AM, kefu chai wrote:
> > On Wed, Nov 15, 2017 at 2:47 PM, kefu chai <tchaikov@gmail.com> wrote:
> > > On Wed, Nov 15, 2017 at 6:04 AM, Ken Dreyer <kdreyer@redhat.com> wrote:
> > > > On Tue, Nov 14, 2017 at 5:49 AM, kefu chai <tchaikov@gmail.com> wrote:
> > > > > On Mon, Nov 13, 2017 at 5:29 PM, kefu chai <tchaikov@gmail.com> wrote:
> > > > > > On Sat, Nov 11, 2017 at 1:47 AM, Sage Weil <sweil@redhat.com> wrote:
> > > > > > > We noticed a big performance regression when switching some code
> > > > > > > to
> > > > > > > use list::size() because although C++11 promizes that it is O(1),
> > > > > > > some of
> > > > > > > the libstdc++'s out there are still O(n).  This PR aims to fix
> > > > > > > that
> > > > > > > 
> > > > > > >          https://github.com/ceph/ceph/pull/18863
> > > > > > > 
> > > > > > > by adding the various devtoolset packages as dependencies to pull
> > > > > > > in
> > > > > > > updated build toolchains.  For el7 that's
> > > > > > > devtoolset-7-{binutils,gcc-c++}.
> > > > > > > 
> > > > > 
> > > > > when running the binaries built using GCC 5.1 in an env where an old
> > > > > libstdc++ (typically comes with GCC 4.8):
> > > > > 
> > > > > $ rados
> > > > > rados: relocation error: /usr/lib/ceph/libceph-common.so.0: symbol
> > > > > _ZTINSt8ios_base7failureB5cxx11E, version GLIBCXX_3.4.21 not defined
> > > > > in file libstdc++.so.6 with link time reference
> > > > > 
> > > > > because libstdc++ introduced a new ABI which is incompatible with the
> > > > > old one.
> > > > > see
> > > > > https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
> > > > > and
> > > > > https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ .
> > > > > 
> > > > > in other words, we need to either 1) link statically with libstdc++.so
> > > > > or
> > > > > 2) include it in librados2 on distros with GCC version less than 5.1.
> > > > > because
> > > > > ceph-osd (and other daemons packages) => ceph-base => ceph-common =>
> > > > > python-rados => librados2.
> > > > 
> > > > For 1), what does that imply for packages' debuginfo sizes?
> > > 
> > > since we don't compile the source of libstdc++, i don't think this
> > > will increase the size of debuginfo.
> > > 
> > > > 
> > > > For 2), where would libstdc++.so ship in the filesystem? What does
> > > > that mean for other applications that would load libstdc++.so and also
> > > > link with ceph?
> > > 
> > > if we just statically link libceph-common against libstdc++, and let
> > > daemons link againt libceph-common, the problem will be solved. that's
> > > actually 2) but after a second thought, i think it's a better
> > > approach. please note, the daemons are linked against libceph-common
> > > statically. [OT] we could let daemon link libceph-common dynamically,
> > > but the downside is that the daemon will need to depend on a client
> > > side library: librados2. which is a little bit weird. but the upside
> > > is: this will shrink the size of debug-info and the size of daemons,
> > > as the daemon will not include ceph-common statically.
> > 
> > i just confirmed with the devtoolset maintainer offline, that the bits
> > not included by
> > the base libstdc++ will be statically linked into the binary. and i
> > verified this behavior
> > on centos7. but this is not true, when it comes to trusty.
> > 
> > 
> > $ bin/rados
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `CXXABI_1.3.8' not found (required by bin/rados)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `CXXABI_1.3.8' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `GLIBCXX_3.4.21' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `GLIBCXX_3.4.22' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/librados.so.2)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `CXXABI_1.3.8' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `GLIBCXX_3.4.21' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `GLIBCXX_3.4.22' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libradosstriper.so.1)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `CXXABI_1.3.8' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `GLIBCXX_3.4.22' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `GLIBCXX_3.4.21' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> > bin/rados: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version
> > `GLIBCXX_3.4.20' not found (required by
> > /var/ceph/ceph-ubuntu-14.04-kefu/build/lib/libceph-common.so.0)
> > 
> > so, an obvious solution is to use "-static-libstdc++"[1] on trusty,
> > libceph-common is a good candidate for where the libstdc++ is
> > included. or just drop the support of it. thoughts?
> > 
> > ---
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html
> > 
> 
> -- 
> Nathan Cutler
> Software Engineer Distributed Storage
> SUSE LINUX, s.r.o.
> Tel.: +420 284 084 037
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-11-30 14:38             ` Sage Weil
@ 2017-12-05 14:53               ` kefu chai
  2017-12-05 15:03                 ` Sage Weil
  2017-12-05 16:40                 ` Adam C. Emerson
  0 siblings, 2 replies; 30+ messages in thread
From: kefu chai @ 2017-12-05 14:53 UTC (permalink / raw)
  To: Sage Weil
  Cc: Nathan Cutler, Ken Dreyer, ceph-maintainers, Alfredo Deza, kchai,
	James Page, ceph-devel

On Thu, Nov 30, 2017 at 10:38 PM, Sage Weil <sweil@redhat.com> wrote:
> On Thu, 30 Nov 2017, Nathan Cutler wrote:
>> I thought we already agreed to drop Trusty in Mimic?
>
> I'm not sure we made any final decision, but I haven't seen anyone
> complain this time around, so I'd say let's do it!

good news, every one! we are now building ceph using gcc-7 on centos7
and trusty.

an example build with gcc-4.8 on trusty:
https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=trusty,DIST=trusty,MACHINE_SIZE=huge/13152//consoleFull
/// we have following warning message:
----8<----
CMake Warning at src/CMakeLists.txt:176 (message):
  performance regression is expected due to an O(n) implementation of
  'std::list::size()' in libstdc++ older than 5.1.0
------>8----
an example build with gcc-7 on trusty:
see https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=trusty,DIST=trusty,MACHINE_SIZE=huge/13156//consoleFull
/// the warning message is gone.

a build with gcc-7 on centos-7,
see https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=centos7,DIST=centos7,MACHINE_SIZE=huge/13155//consoleFull

once all trusty testnodes are re-imaged with xenial, i will try to
remove the trusty build support on ceph/ceph-build. BTW, if we want to
use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
it also.

the only issue is http://tracker.ceph.com/issues/22301. its fix is
pending on review.

-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 14:53               ` kefu chai
@ 2017-12-05 15:03                 ` Sage Weil
  2017-12-05 15:08                   ` kefu chai
  2017-12-05 16:40                 ` Adam C. Emerson
  1 sibling, 1 reply; 30+ messages in thread
From: Sage Weil @ 2017-12-05 15:03 UTC (permalink / raw)
  To: kefu chai
  Cc: Nathan Cutler, Ken Dreyer, ceph-maintainers, Alfredo Deza, kchai,
	James Page, ceph-devel

On Tue, 5 Dec 2017, kefu chai wrote:
> On Thu, Nov 30, 2017 at 10:38 PM, Sage Weil <sweil@redhat.com> wrote:
> > On Thu, 30 Nov 2017, Nathan Cutler wrote:
> >> I thought we already agreed to drop Trusty in Mimic?
> >
> > I'm not sure we made any final decision, but I haven't seen anyone
> > complain this time around, so I'd say let's do it!
> 
> good news, every one! we are now building ceph using gcc-7 on centos7
> and trusty.
> 
> an example build with gcc-4.8 on trusty:
> https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=trusty,DIST=trusty,MACHINE_SIZE=huge/13152//consoleFull
> /// we have following warning message:
> ----8<----
> CMake Warning at src/CMakeLists.txt:176 (message):
>   performance regression is expected due to an O(n) implementation of
>   'std::list::size()' in libstdc++ older than 5.1.0
> ------>8----
> an example build with gcc-7 on trusty:
> see https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=trusty,DIST=trusty,MACHINE_SIZE=huge/13156//consoleFull
> /// the warning message is gone.
>
> a build with gcc-7 on centos-7,
> see https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=centos7,DIST=centos7,MACHINE_SIZE=huge/13155//consoleFull

Yay!!

> once all trusty testnodes are re-imaged with xenial, i will try to
> remove the trusty build support on ceph/ceph-build. BTW, if we want to
> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
> it also.

I think we need to keep the trusty build support there for luminous and 
jewel builds, right?

> the only issue is http://tracker.ceph.com/issues/22301. its fix is
> pending on review.
> 
> -- 
> Regards
> Kefu Chai
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 15:03                 ` Sage Weil
@ 2017-12-05 15:08                   ` kefu chai
  0 siblings, 0 replies; 30+ messages in thread
From: kefu chai @ 2017-12-05 15:08 UTC (permalink / raw)
  To: Sage Weil
  Cc: Nathan Cutler, Ken Dreyer, ceph-maintainers, Alfredo Deza, kchai,
	James Page, ceph-devel

On Tue, Dec 5, 2017 at 11:03 PM, Sage Weil <sweil@redhat.com> wrote:
> On Tue, 5 Dec 2017, kefu chai wrote:
>> On Thu, Nov 30, 2017 at 10:38 PM, Sage Weil <sweil@redhat.com> wrote:
>> > On Thu, 30 Nov 2017, Nathan Cutler wrote:
>> >> I thought we already agreed to drop Trusty in Mimic?
>> >
>> > I'm not sure we made any final decision, but I haven't seen anyone
>> > complain this time around, so I'd say let's do it!
>>
>> good news, every one! we are now building ceph using gcc-7 on centos7
>> and trusty.
>>
>> an example build with gcc-4.8 on trusty:
>> https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=trusty,DIST=trusty,MACHINE_SIZE=huge/13152//consoleFull
>> /// we have following warning message:
>> ----8<----
>> CMake Warning at src/CMakeLists.txt:176 (message):
>>   performance regression is expected due to an O(n) implementation of
>>   'std::list::size()' in libstdc++ older than 5.1.0
>> ------>8----
>> an example build with gcc-7 on trusty:
>> see https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=trusty,DIST=trusty,MACHINE_SIZE=huge/13156//consoleFull
>> /// the warning message is gone.
>>
>> a build with gcc-7 on centos-7,
>> see https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=centos7,DIST=centos7,MACHINE_SIZE=huge/13155//consoleFull
>
> Yay!!
>
>> once all trusty testnodes are re-imaged with xenial, i will try to
>> remove the trusty build support on ceph/ceph-build. BTW, if we want to
>> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
>> it also.
>
> I think we need to keep the trusty build support there for luminous and
> jewel builds, right?


gahhh! i completely neglected them! then i will need to build them
using the old toolchain, otherwise they will fail at run-time, due to
missing libstdc++ ABI version! will send a PR on ceph/ceph-build
tomorrow.

>
>> the only issue is http://tracker.ceph.com/issues/22301. its fix is
>> pending on review.
>>
>> --
>> Regards
>> Kefu Chai
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>



-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 14:53               ` kefu chai
  2017-12-05 15:03                 ` Sage Weil
@ 2017-12-05 16:40                 ` Adam C. Emerson
  2017-12-05 16:51                   ` Jesse Williamson
                                     ` (2 more replies)
  1 sibling, 3 replies; 30+ messages in thread
From: Adam C. Emerson @ 2017-12-05 16:40 UTC (permalink / raw)
  To: kefu chai
  Cc: Sage Weil, Nathan Cutler, Ken Dreyer, ceph-maintainers,
	Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On 05/12/2017, kefu chai wrote:
> once all trusty testnodes are re-imaged with xenial, i will try to
> remove the trusty build support on ceph/ceph-build. BTW, if we want to
> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
> it also.

If we can get GCC-7 on Xenial and there's a way for people building
Ceph to get it without too much trouble, then I think we should use
GCC-7.

In general, so long as they are reasonably gettable on our target
operating systems, I think we ought to target the latest stable
release stream of GCC and Clang.

-- 
Senior Software Engineer           Red Hat Storage, Ann Arbor, MI, US
IRC: Aemerson@OFTC, Actinic@Freenode
0x80F7544B90EDBFB9 E707 86BA 0C1B 62CC 152C  7C12 80F7 544B 90ED BFB9

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 16:40                 ` Adam C. Emerson
@ 2017-12-05 16:51                   ` Jesse Williamson
  2017-12-05 16:58                   ` Willem Jan Withagen
  2017-12-05 22:23                   ` Nathan Cutler
  2 siblings, 0 replies; 30+ messages in thread
From: Jesse Williamson @ 2017-12-05 16:51 UTC (permalink / raw)
  To: Adam C. Emerson
  Cc: kefu chai, Sage Weil, Nathan Cutler, Ken Dreyer,
	ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Tue, 5 Dec 2017, Adam C. Emerson wrote:

> In general, so long as they are reasonably gettable on our target
> operating systems, I think we ought to target the latest stable
> release stream of GCC and Clang.

Much +1 here-- not only for access to "shiny and new", but also
important fixes in language support, etc..

-Jesse

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 16:40                 ` Adam C. Emerson
  2017-12-05 16:51                   ` Jesse Williamson
@ 2017-12-05 16:58                   ` Willem Jan Withagen
  2017-12-05 17:51                     ` Gregory Farnum
  2017-12-05 22:23                   ` Nathan Cutler
  2 siblings, 1 reply; 30+ messages in thread
From: Willem Jan Withagen @ 2017-12-05 16:58 UTC (permalink / raw)
  To: kefu chai, Sage Weil, Nathan Cutler, Ken Dreyer,
	ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On 5-12-2017 17:40, Adam C. Emerson wrote:
> On 05/12/2017, kefu chai wrote:
>> once all trusty testnodes are re-imaged with xenial, i will try to
>> remove the trusty build support on ceph/ceph-build. BTW, if we want to
>> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
>> it also.
> 
> If we can get GCC-7 on Xenial and there's a way for people building
> Ceph to get it without too much trouble, then I think we should use
> GCC-7.
> 
> In general, so long as they are reasonably gettable on our target
> operating systems, I think we ought to target the latest stable
> release stream of GCC and Clang.
> 

"somebody" dropped the Clang word. :)

Dare I then suggest that some of the build be done against Clang whilest 
building PRs also.

--WjW


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 16:58                   ` Willem Jan Withagen
@ 2017-12-05 17:51                     ` Gregory Farnum
  2017-12-06  9:05                       ` Willem Jan Withagen
  0 siblings, 1 reply; 30+ messages in thread
From: Gregory Farnum @ 2017-12-05 17:51 UTC (permalink / raw)
  To: Willem Jan Withagen
  Cc: kefu chai, Sage Weil, Nathan Cutler, Ken Dreyer,
	ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Tue, Dec 5, 2017 at 8:58 AM, Willem Jan Withagen <wjw@digiware.nl> wrote:
> On 5-12-2017 17:40, Adam C. Emerson wrote:
>>
>> On 05/12/2017, kefu chai wrote:
>>>
>>> once all trusty testnodes are re-imaged with xenial, i will try to
>>> remove the trusty build support on ceph/ceph-build. BTW, if we want to
>>> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
>>> it also.
>>
>>
>> If we can get GCC-7 on Xenial and there's a way for people building
>> Ceph to get it without too much trouble, then I think we should use
>> GCC-7.
>>
>> In general, so long as they are reasonably gettable on our target
>> operating systems, I think we ought to target the latest stable
>> release stream of GCC and Clang.

I was talking with Josh yesterday and he mentioned how newer C++
releases have a less stable ABI and we often need to ship out
libstdc++ with them. Would this change anything we care about?
(I, like, super don't care. But I also don't really know anything about it.)


> "somebody" dropped the Clang word. :)
>
> Dare I then suggest that some of the build be done against Clang whilest
> building PRs also.

I think we'd all love this. Maybe somebody with a vested interest in
Clang working properly and recent experience mucking around with our
build systems will submit a PR? :p
-Greg

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 16:40                 ` Adam C. Emerson
  2017-12-05 16:51                   ` Jesse Williamson
  2017-12-05 16:58                   ` Willem Jan Withagen
@ 2017-12-05 22:23                   ` Nathan Cutler
  2017-12-05 22:41                     ` Adam C. Emerson
  2 siblings, 1 reply; 30+ messages in thread
From: Nathan Cutler @ 2017-12-05 22:23 UTC (permalink / raw)
  To: kefu chai, Sage Weil, Ken Dreyer, ceph-maintainers, Alfredo Deza,
	James Page, The Esoteric Order of the Squid Cybernetic

On 12/05/2017 05:40 PM, Adam C. Emerson wrote:
> On 05/12/2017, kefu chai wrote:
>> once all trusty testnodes are re-imaged with xenial, i will try to
>> remove the trusty build support on ceph/ceph-build. BTW, if we want to
>> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
>> it also.
> 
> If we can get GCC-7 on Xenial and there's a way for people building
> Ceph to get it without too much trouble, then I think we should use
> GCC-7.
> 
> In general, so long as they are reasonably gettable on our target
> operating systems, I think we ought to target the latest stable
> release stream of GCC and Clang.

This sounds like it is edging towards requiring GCC-7 to build 
master/Mimic? That seems kind of radical, but I don't see it posing any 
problem for openSUSE or SLES since I'm already using GCC-7 to build 
master/Mimic in OBS.

Nathan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 22:23                   ` Nathan Cutler
@ 2017-12-05 22:41                     ` Adam C. Emerson
  0 siblings, 0 replies; 30+ messages in thread
From: Adam C. Emerson @ 2017-12-05 22:41 UTC (permalink / raw)
  To: Nathan Cutler
  Cc: kefu chai, Sage Weil, Ken Dreyer, ceph-maintainers, Alfredo Deza,
	James Page, The Esoteric Order of the Squid Cybernetic

On 05/12/2017, Nathan Cutler wrote:
[snip]
> This sounds like it is edging towards requiring GCC-7 to build master/Mimic?
> That seems kind of radical, but I don't see it posing any problem for
> openSUSE or SLES since I'm already using GCC-7 to build master/Mimic in OBS.

It is. Well, I didn't intend to edge toward it. I intended to suggest
outright:

If our target platforms have versions of GCC 7 and/or Clang 5 that we
can reasonably expect people to install, (and it looked like that's
the case for both CentOS and Ubuntu), then we should target GCC 7 and
Clang 5.

Having a narrower range of compilers we promise to support should
generally make our lives easier.

-- 
Senior Software Engineer           Red Hat Storage, Ann Arbor, MI, US
IRC: Aemerson@OFTC, Actinic@Freenode
0x80F7544B90EDBFB9 E707 86BA 0C1B 62CC 152C  7C12 80F7 544B 90ED BFB9

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-05 17:51                     ` Gregory Farnum
@ 2017-12-06  9:05                       ` Willem Jan Withagen
  2017-12-23  9:46                         ` Xuehan Xu
  0 siblings, 1 reply; 30+ messages in thread
From: Willem Jan Withagen @ 2017-12-06  9:05 UTC (permalink / raw)
  To: Gregory Farnum
  Cc: kefu chai, Sage Weil, Nathan Cutler, Ken Dreyer,
	ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On 05/12/2017 18:51, Gregory Farnum wrote:
> On Tue, Dec 5, 2017 at 8:58 AM, Willem Jan Withagen <wjw@digiware.nl> wrote:
>> On 5-12-2017 17:40, Adam C. Emerson wrote:
>>>
>>> On 05/12/2017, kefu chai wrote:
>>>>
>>>> once all trusty testnodes are re-imaged with xenial, i will try to
>>>> remove the trusty build support on ceph/ceph-build. BTW, if we want to
>>>> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
>>>> it also.
>>>
>>>
>>> If we can get GCC-7 on Xenial and there's a way for people building
>>> Ceph to get it without too much trouble, then I think we should use
>>> GCC-7.
>>>
>>> In general, so long as they are reasonably gettable on our target
>>> operating systems, I think we ought to target the latest stable
>>> release stream of GCC and Clang.
> 
> I was talking with Josh yesterday and he mentioned how newer C++
> releases have a less stable ABI and we often need to ship out
> libstdc++ with them. Would this change anything we care about?
> (I, like, super don't care. But I also don't really know anything about it.)
> 
> 
>> "somebody" dropped the Clang word. :)
>>
>> Dare I then suggest that some of the build be done against Clang whilest
>> building PRs also.
> 
> I think we'd all love this. Maybe somebody with a vested interest in
> Clang working properly and recent experience mucking around with our
> build systems will submit a PR? :p

Yeah, I know.... Put the code where your mouth is....

Uptill now I've avoided getting dragged into too much knowledge about 
exactly this part. I'd be willing to deliver FreeBSD/Clang knowledge for 
that. But unfortunately I do not have enough time to start digging into 
this.

--WjW


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-06  9:05                       ` Willem Jan Withagen
@ 2017-12-23  9:46                         ` Xuehan Xu
  2017-12-25 14:43                           ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: Xuehan Xu @ 2017-12-23  9:46 UTC (permalink / raw)
  To: Willem Jan Withagen
  Cc: Gregory Farnum, kefu chai, Sage Weil, Nathan Cutler, Ken Dreyer,
	ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

Hi, everyone.

I'm using CentOS 7 with devtoolset-7 to build ceph. However, I found
that the list::size() in my ceph executables is still counting the
number of nodes. It's so odd, I googled it, and found that this may
have something to do with _GLIBCXX_USE_CXX11_ABI. But even if I
explicitly set _GLIBCXX_USE_CXX11_ABI to 1 when building ceph, the
problem remains. It seems that _GLIBCXX_USE_CXX11_ABI is set to 0 by
gcc no matter how I build the source code, even with
-D_GLIBCXX_USE_CXX11_ABI=1.


Does anyone have any clue? Thanks:-)

On 6 December 2017 at 04:05, Willem Jan Withagen <wjw@digiware.nl> wrote:
> On 05/12/2017 18:51, Gregory Farnum wrote:
>>
>> On Tue, Dec 5, 2017 at 8:58 AM, Willem Jan Withagen <wjw@digiware.nl>
>> wrote:
>>>
>>> On 5-12-2017 17:40, Adam C. Emerson wrote:
>>>>
>>>>
>>>> On 05/12/2017, kefu chai wrote:
>>>>>
>>>>>
>>>>> once all trusty testnodes are re-imaged with xenial, i will try to
>>>>> remove the trusty build support on ceph/ceph-build. BTW, if we want to
>>>>> use gcc-7 on xenial, which ships GCC 5.3, we can apply the same fix on
>>>>> it also.
>>>>
>>>>
>>>>
>>>> If we can get GCC-7 on Xenial and there's a way for people building
>>>> Ceph to get it without too much trouble, then I think we should use
>>>> GCC-7.
>>>>
>>>> In general, so long as they are reasonably gettable on our target
>>>> operating systems, I think we ought to target the latest stable
>>>> release stream of GCC and Clang.
>>
>>
>> I was talking with Josh yesterday and he mentioned how newer C++
>> releases have a less stable ABI and we often need to ship out
>> libstdc++ with them. Would this change anything we care about?
>> (I, like, super don't care. But I also don't really know anything about
>> it.)
>>
>>
>>> "somebody" dropped the Clang word. :)
>>>
>>> Dare I then suggest that some of the build be done against Clang whilest
>>> building PRs also.
>>
>>
>> I think we'd all love this. Maybe somebody with a vested interest in
>> Clang working properly and recent experience mucking around with our
>> build systems will submit a PR? :p
>
>
> Yeah, I know.... Put the code where your mouth is....
>
> Uptill now I've avoided getting dragged into too much knowledge about
> exactly this part. I'd be willing to deliver FreeBSD/Clang knowledge for
> that. But unfortunately I do not have enough time to start digging into
> this.
>
> --WjW
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-23  9:46                         ` Xuehan Xu
@ 2017-12-25 14:43                           ` kefu chai
  2017-12-25 16:38                             ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: kefu chai @ 2017-12-25 14:43 UTC (permalink / raw)
  To: Xuehan Xu
  Cc: Willem Jan Withagen, Gregory Farnum, Sage Weil, Nathan Cutler,
	Ken Dreyer, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Sat, Dec 23, 2017 at 5:46 PM, Xuehan Xu <xxhdx1985126@gmail.com> wrote:
> Hi, everyone.
>
> I'm using CentOS 7 with devtoolset-7 to build ceph. However, I found
> that the list::size() in my ceph executables is still counting the
> number of nodes. It's so odd, I googled it, and found that this may
> have something to do with _GLIBCXX_USE_CXX11_ABI. But even if I
> explicitly set _GLIBCXX_USE_CXX11_ABI to 1 when building ceph, the
> problem remains. It seems that _GLIBCXX_USE_CXX11_ABI is set to 0 by
> gcc no matter how I build the source code, even with
> -D_GLIBCXX_USE_CXX11_ABI=1.

Xuehan, thanks for testing!

turns out the gcc-7 in devtoolset is compiled with
"--with-default-libstdcxx-abi=gcc4-compatible", see the output of "g++
-v", so it is using "_GLIBCXX_USE_CXX11_ABI=0", you can by checking
/opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h
.

in other words, we still need https://github.com/ceph/ceph/pull/18755.

>
>
> Does anyone have any clue? Thanks:-)
>

-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-25 14:43                           ` kefu chai
@ 2017-12-25 16:38                             ` kefu chai
  2017-12-26  6:35                               ` Xuehan Xu
  0 siblings, 1 reply; 30+ messages in thread
From: kefu chai @ 2017-12-25 16:38 UTC (permalink / raw)
  To: Xuehan Xu
  Cc: Willem Jan Withagen, Gregory Farnum, Sage Weil, Nathan Cutler,
	Ken Dreyer, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Mon, Dec 25, 2017 at 10:43 PM, kefu chai <tchaikov@gmail.com> wrote:
> On Sat, Dec 23, 2017 at 5:46 PM, Xuehan Xu <xxhdx1985126@gmail.com> wrote:
>> Hi, everyone.
>>
>> I'm using CentOS 7 with devtoolset-7 to build ceph. However, I found
>> that the list::size() in my ceph executables is still counting the
>> number of nodes. It's so odd, I googled it, and found that this may
>> have something to do with _GLIBCXX_USE_CXX11_ABI. But even if I
>> explicitly set _GLIBCXX_USE_CXX11_ABI to 1 when building ceph, the
>> problem remains. It seems that _GLIBCXX_USE_CXX11_ABI is set to 0 by
>> gcc no matter how I build the source code, even with
>> -D_GLIBCXX_USE_CXX11_ABI=1.
>
> Xuehan, thanks for testing!
>
> turns out the gcc-7 in devtoolset is compiled with
> "--with-default-libstdcxx-abi=gcc4-compatible", see the output of "g++
> -v", so it is using "_GLIBCXX_USE_CXX11_ABI=0", you can by checking
> /opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h
> .
>
> in other words, we still need https://github.com/ceph/ceph/pull/18755.


hmm, probably not.

 add_definitions(-D_GLIBCXX_USE_CXX11_ABI=1)

and

 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fabi-version=0")

might help. I will give it a try tomorrow.



-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-25 16:38                             ` kefu chai
@ 2017-12-26  6:35                               ` Xuehan Xu
  2017-12-26 11:24                                 ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: Xuehan Xu @ 2017-12-26  6:35 UTC (permalink / raw)
  To: kefu chai
  Cc: Willem Jan Withagen, Gregory Farnum, Sage Weil, Nathan Cutler,
	Ken Dreyer, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

Thanks for your reply, kefu:-)

I checked /opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h,
and saw the following configuration:

# define _GLIBCXX_USE_DUAL_ABI 0

#if ! _GLIBCXX_USE_DUAL_ABI
// Ignore any pre-defined value of _GLIBCXX_USE_CXX11_ABI
# undef _GLIBCXX_USE_CXX11_ABI
#endif

#ifndef _GLIBCXX_USE_CXX11_ABI
# define _GLIBCXX_USE_CXX11_ABI 0
#endif

It seems that no matter how we config MAKEFILE or CMakeFile,
_GLIBCXX_USE_CXX11_ABI would always be 0 under the circumstance.
So I think https://github.com/ceph/ceph/pull/18755 is still needed.

Is this right?

On 26 December 2017 at 00:38, kefu chai <tchaikov@gmail.com> wrote:
> On Mon, Dec 25, 2017 at 10:43 PM, kefu chai <tchaikov@gmail.com> wrote:
>> On Sat, Dec 23, 2017 at 5:46 PM, Xuehan Xu <xxhdx1985126@gmail.com> wrote:
>>> Hi, everyone.
>>>
>>> I'm using CentOS 7 with devtoolset-7 to build ceph. However, I found
>>> that the list::size() in my ceph executables is still counting the
>>> number of nodes. It's so odd, I googled it, and found that this may
>>> have something to do with _GLIBCXX_USE_CXX11_ABI. But even if I
>>> explicitly set _GLIBCXX_USE_CXX11_ABI to 1 when building ceph, the
>>> problem remains. It seems that _GLIBCXX_USE_CXX11_ABI is set to 0 by
>>> gcc no matter how I build the source code, even with
>>> -D_GLIBCXX_USE_CXX11_ABI=1.
>>
>> Xuehan, thanks for testing!
>>
>> turns out the gcc-7 in devtoolset is compiled with
>> "--with-default-libstdcxx-abi=gcc4-compatible", see the output of "g++
>> -v", so it is using "_GLIBCXX_USE_CXX11_ABI=0", you can by checking
>> /opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h
>> .
>>
>> in other words, we still need https://github.com/ceph/ceph/pull/18755.
>
>
> hmm, probably not.
>
>  add_definitions(-D_GLIBCXX_USE_CXX11_ABI=1)
>
> and
>
>  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fabi-version=0")
>
> might help. I will give it a try tomorrow.
>
>
>
> --
> Regards
> Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-26  6:35                               ` Xuehan Xu
@ 2017-12-26 11:24                                 ` kefu chai
  2017-12-30  3:37                                   ` Xuehan Xu
  2018-01-02 15:28                                   ` Ken Dreyer
  0 siblings, 2 replies; 30+ messages in thread
From: kefu chai @ 2017-12-26 11:24 UTC (permalink / raw)
  To: Xuehan Xu
  Cc: Willem Jan Withagen, Gregory Farnum, Sage Weil, Nathan Cutler,
	Ken Dreyer, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

xuehan, could you avoid top-posting? otherwise other subscribers will
have trouble to get context of the discussion if he/she fail to read
the whole thread.

On Tue, Dec 26, 2017 at 2:35 PM, Xuehan Xu <xxhdx1985126@gmail.com> wrote:
> Thanks for your reply, kefu:-)
>
> I checked /opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h,
> and saw the following configuration:
>
> # define _GLIBCXX_USE_DUAL_ABI 0

yes. this is the crux of the problem, see
https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/2e5ef6c7934d4417e095855478736742b35bd0af/SPECS!gcc.spec#L1026
. so,

- “--with-default-libstdcxx-abi=gcc4-compatible”, the DTS-7's
libstdc++'s default ABI is the old one, and
- it does *not* use dual ABI.

hence what we have is the gcc4 compatible ABI, and nothing more, on centos 7.

>
> #if ! _GLIBCXX_USE_DUAL_ABI
> // Ignore any pre-defined value of _GLIBCXX_USE_CXX11_ABI
> # undef _GLIBCXX_USE_CXX11_ABI
> #endif
>
> #ifndef _GLIBCXX_USE_CXX11_ABI
> # define _GLIBCXX_USE_CXX11_ABI 0
> #endif
>
> It seems that no matter how we config MAKEFILE or CMakeFile,
> _GLIBCXX_USE_CXX11_ABI would always be 0 under the circumstance.
> So I think https://github.com/ceph/ceph/pull/18755 is still needed.
>
> Is this right?

right. unless we swing our own homebrew gcc-7.

>
> On 26 December 2017 at 00:38, kefu chai <tchaikov@gmail.com> wrote:
>> On Mon, Dec 25, 2017 at 10:43 PM, kefu chai <tchaikov@gmail.com> wrote:
>>> On Sat, Dec 23, 2017 at 5:46 PM, Xuehan Xu <xxhdx1985126@gmail.com> wrote:
>>>> Hi, everyone.
>>>>
>>>> I'm using CentOS 7 with devtoolset-7 to build ceph. However, I found
>>>> that the list::size() in my ceph executables is still counting the
>>>> number of nodes. It's so odd, I googled it, and found that this may
>>>> have something to do with _GLIBCXX_USE_CXX11_ABI. But even if I
>>>> explicitly set _GLIBCXX_USE_CXX11_ABI to 1 when building ceph, the
>>>> problem remains. It seems that _GLIBCXX_USE_CXX11_ABI is set to 0 by
>>>> gcc no matter how I build the source code, even with
>>>> -D_GLIBCXX_USE_CXX11_ABI=1.
>>>
>>> Xuehan, thanks for testing!
>>>
>>> turns out the gcc-7 in devtoolset is compiled with
>>> "--with-default-libstdcxx-abi=gcc4-compatible", see the output of "g++
>>> -v", so it is using "_GLIBCXX_USE_CXX11_ABI=0", you can by checking
>>> /opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h
>>> .
>>>
>>> in other words, we still need https://github.com/ceph/ceph/pull/18755.
>>
>>
>> hmm, probably not.
>>
>>  add_definitions(-D_GLIBCXX_USE_CXX11_ABI=1)
>>
>> and
>>
>>  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fabi-version=0")
>>
>> might help. I will give it a try tomorrow.

per the discussion above. we still need it. list::size() is O(n) in
libstdc++ offered by DTS-7.


-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-26 11:24                                 ` kefu chai
@ 2017-12-30  3:37                                   ` Xuehan Xu
  2018-01-02 15:28                                   ` Ken Dreyer
  1 sibling, 0 replies; 30+ messages in thread
From: Xuehan Xu @ 2017-12-30  3:37 UTC (permalink / raw)
  To: kefu chai
  Cc: Willem Jan Withagen, Gregory Farnum, Sage Weil, Nathan Cutler,
	Ken Dreyer, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On 26 December 2017 at 19:24, kefu chai <tchaikov@gmail.com> wrote:
> xuehan, could you avoid top-posting? otherwise other subscribers will
> have trouble to get context of the discussion if he/she fail to read
> the whole thread.
>
> On Tue, Dec 26, 2017 at 2:35 PM, Xuehan Xu <xxhdx1985126@gmail.com> wrote:
>> Thanks for your reply, kefu:-)
>>
>> I checked /opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h,
>> and saw the following configuration:
>>
>> # define _GLIBCXX_USE_DUAL_ABI 0
>
> yes. this is the crux of the problem, see
> https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/2e5ef6c7934d4417e095855478736742b35bd0af/SPECS!gcc.spec#L1026
> . so,
>
> - “--with-default-libstdcxx-abi=gcc4-compatible”, the DTS-7's
> libstdc++'s default ABI is the old one, and
> - it does *not* use dual ABI.
>
> hence what we have is the gcc4 compatible ABI, and nothing more, on centos 7.
>
>>
>> #if ! _GLIBCXX_USE_DUAL_ABI
>> // Ignore any pre-defined value of _GLIBCXX_USE_CXX11_ABI
>> # undef _GLIBCXX_USE_CXX11_ABI
>> #endif
>>
>> #ifndef _GLIBCXX_USE_CXX11_ABI
>> # define _GLIBCXX_USE_CXX11_ABI 0
>> #endif
>>
>> It seems that no matter how we config MAKEFILE or CMakeFile,
>> _GLIBCXX_USE_CXX11_ABI would always be 0 under the circumstance.
>> So I think https://github.com/ceph/ceph/pull/18755 is still needed.
>>
>> Is this right?
>
> right. unless we swing our own homebrew gcc-7.
>
>>
>> On 26 December 2017 at 00:38, kefu chai <tchaikov@gmail.com> wrote:
>>> On Mon, Dec 25, 2017 at 10:43 PM, kefu chai <tchaikov@gmail.com> wrote:
>>>> On Sat, Dec 23, 2017 at 5:46 PM, Xuehan Xu <xxhdx1985126@gmail.com> wrote:
>>>>> Hi, everyone.
>>>>>
>>>>> I'm using CentOS 7 with devtoolset-7 to build ceph. However, I found
>>>>> that the list::size() in my ceph executables is still counting the
>>>>> number of nodes. It's so odd, I googled it, and found that this may
>>>>> have something to do with _GLIBCXX_USE_CXX11_ABI. But even if I
>>>>> explicitly set _GLIBCXX_USE_CXX11_ABI to 1 when building ceph, the
>>>>> problem remains. It seems that _GLIBCXX_USE_CXX11_ABI is set to 0 by
>>>>> gcc no matter how I build the source code, even with
>>>>> -D_GLIBCXX_USE_CXX11_ABI=1.
>>>>
>>>> Xuehan, thanks for testing!
>>>>
>>>> turns out the gcc-7 in devtoolset is compiled with
>>>> "--with-default-libstdcxx-abi=gcc4-compatible", see the output of "g++
>>>> -v", so it is using "_GLIBCXX_USE_CXX11_ABI=0", you can by checking
>>>> /opt/rh/devtoolset-7/root/usr/include/c++/7/x86_64-redhat-linux/bits/c++config.h
>>>> .
>>>>
>>>> in other words, we still need https://github.com/ceph/ceph/pull/18755.
>>>
>>>
>>> hmm, probably not.
>>>
>>>  add_definitions(-D_GLIBCXX_USE_CXX11_ABI=1)
>>>
>>> and
>>>
>>>  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fabi-version=0")
>>>
>>> might help. I will give it a try tomorrow.
>
> per the discussion above. we still need it. list::size() is O(n) in
> libstdc++ offered by DTS-7.
>
>
> --
> Regards
> Kefu Chai
Hi, I took a look at https://github.com/ceph/ceph/pull/18755. It seems
that it only removes the pg_log.size()'s invocation, however,
according to our test, as discusse in
https://www.spinics.net/lists/ceph-devel/msg39288.html, there's
another place where list::size() can lead to significant performance
regression: SimpleLRU, as list::size() is invoked to compute the
cache's size when trimming SimpleLRU cache. So I updated the PR
https://github.com/ceph/ceph/pull/19397 with a new commit that
manually count the size of the cache relying on list::size().

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2017-12-26 11:24                                 ` kefu chai
  2017-12-30  3:37                                   ` Xuehan Xu
@ 2018-01-02 15:28                                   ` Ken Dreyer
  2018-01-02 15:45                                     ` kefu chai
  1 sibling, 1 reply; 30+ messages in thread
From: Ken Dreyer @ 2018-01-02 15:28 UTC (permalink / raw)
  To: kefu chai
  Cc: Xuehan Xu, Willem Jan Withagen, Gregory Farnum, Sage Weil,
	Nathan Cutler, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Tue, Dec 26, 2017 at 4:24 AM, kefu chai <tchaikov@gmail.com> wrote:
> right. unless we swing our own homebrew gcc-7.

Can we ask the devtoolset maintainers to change this option?

I imagine they would be interested in this type of feedback from the field.

- Ken

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2018-01-02 15:28                                   ` Ken Dreyer
@ 2018-01-02 15:45                                     ` kefu chai
  2018-01-02 17:31                                       ` Sage Weil
  0 siblings, 1 reply; 30+ messages in thread
From: kefu chai @ 2018-01-02 15:45 UTC (permalink / raw)
  To: Ken Dreyer
  Cc: Xuehan Xu, Willem Jan Withagen, Gregory Farnum, Sage Weil,
	Nathan Cutler, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Tue, Jan 2, 2018 at 11:28 PM, Ken Dreyer <kdreyer@redhat.com> wrote:
> On Tue, Dec 26, 2017 at 4:24 AM, kefu chai <tchaikov@gmail.com> wrote:
>> right. unless we swing our own homebrew gcc-7.
>
> Can we ask the devtoolset maintainers to change this option?
>
> I imagine they would be interested in this type of feedback from the field.

https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/2e5ef6c7934d4417e095855478736742b35bd0af/SPECS!gcc.spec#L1026

I think we need to think about this before sending any inquiries to them.

-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2018-01-02 15:45                                     ` kefu chai
@ 2018-01-02 17:31                                       ` Sage Weil
  2018-01-03 14:53                                         ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: Sage Weil @ 2018-01-02 17:31 UTC (permalink / raw)
  To: kefu chai
  Cc: Ken Dreyer, Xuehan Xu, Willem Jan Withagen, Gregory Farnum,
	Nathan Cutler, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Tue, 2 Jan 2018, kefu chai wrote:
> On Tue, Jan 2, 2018 at 11:28 PM, Ken Dreyer <kdreyer@redhat.com> wrote:
> > On Tue, Dec 26, 2017 at 4:24 AM, kefu chai <tchaikov@gmail.com> wrote:
> >> right. unless we swing our own homebrew gcc-7.
> >
> > Can we ask the devtoolset maintainers to change this option?
> >
> > I imagine they would be interested in this type of feedback from the field.
> 
> https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/2e5ef6c7934d4417e095855478736742b35bd0af/SPECS!gcc.spec#L1026

For reference,

```
  # Force the old ABI unconditionally, the new one does not work in the
  # libstdc++_nonshared.a model against RHEL 6/7 libstdc++.so.6.
  sed -i -e 's/\(define[[:blank:]]*_GLIBCXX_USE_DUAL_ABI[[:blank:]]*\)1/\10/' $f
```

> I think we need to think about this before sending any inquiries to them.

The problem I see is that it is impossible to write/build many native 
C++11 (or 14 or 17) apps with the toolchain since the old ABI precludes 
O(1) std::list.  

If I'm following the _nonshared thing, it's a feature that lets you build 
with teh new toolchain but deploy on systems with much older libstdc++ 
installed (new symbols are statically instead of dynamically linked).  
Is the problem with the "dual ABI" feature?

sage

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2018-01-02 17:31                                       ` Sage Weil
@ 2018-01-03 14:53                                         ` kefu chai
  2018-01-10  9:46                                           ` kefu chai
  0 siblings, 1 reply; 30+ messages in thread
From: kefu chai @ 2018-01-03 14:53 UTC (permalink / raw)
  To: Sage Weil
  Cc: Ken Dreyer, Xuehan Xu, Willem Jan Withagen, Gregory Farnum,
	Nathan Cutler, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Wed, Jan 3, 2018 at 1:31 AM, Sage Weil <sweil@redhat.com> wrote:
> On Tue, 2 Jan 2018, kefu chai wrote:
>> On Tue, Jan 2, 2018 at 11:28 PM, Ken Dreyer <kdreyer@redhat.com> wrote:
>> > On Tue, Dec 26, 2017 at 4:24 AM, kefu chai <tchaikov@gmail.com> wrote:
>> >> right. unless we swing our own homebrew gcc-7.
>> >
>> > Can we ask the devtoolset maintainers to change this option?
>> >
>> > I imagine they would be interested in this type of feedback from the field.
>>
>> https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/2e5ef6c7934d4417e095855478736742b35bd0af/SPECS!gcc.spec#L1026
>
> For reference,
>
> ```
>   # Force the old ABI unconditionally, the new one does not work in the
>   # libstdc++_nonshared.a model against RHEL 6/7 libstdc++.so.6.
>   sed -i -e 's/\(define[[:blank:]]*_GLIBCXX_USE_DUAL_ABI[[:blank:]]*\)1/\10/' $f
> ```
>
>> I think we need to think about this before sending any inquiries to them.
>
> The problem I see is that it is impossible to write/build many native
> C++11 (or 14 or 17) apps with the toolchain since the old ABI precludes
> O(1) std::list.

i sort of agree with you. but i think we can still use C++11 (and
probably 14 or 17)
with the old ABI. see
https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html

<quote>
the choice of ABI to use is independent of the -std option used to
compile your code
</quote>

in other words, if we rely on the O(1) std::list and copy-on-write
string, then, yes,
we cannot write/build apps using this toolchain.

>
> If I'm following the _nonshared thing, it's a feature that lets you build
> with teh new toolchain but deploy on systems with much older libstdc++
> installed (new symbols are statically instead of dynamically linked).
> Is the problem with the "dual ABI" feature?

i think so. also by inspecting
https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/c7/SOURCES!gcc7-libstdc++-compat.patch,
libstdc++_nonshared.a includes lots of C++98 and C++11 symbols. in the
case of list::size(), it is completely implemented in the header file.
but probably there are also some pieces of libstdc++ implemented in
the source files. in that case, it would be difficult to include both
ABIs a class, std::basic_string<> for example, in the same .a file.
because they are conditionalized by the "_GLIBCXX_USE_CXX11_ABI" macro
at build time. and they are both in the namespace of std::__cxx11
namespace.

will drop a mail to SCLs asking for more info on this.


-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: C++11, std::list::size(), and trusty
  2018-01-03 14:53                                         ` kefu chai
@ 2018-01-10  9:46                                           ` kefu chai
  0 siblings, 0 replies; 30+ messages in thread
From: kefu chai @ 2018-01-10  9:46 UTC (permalink / raw)
  To: Sage Weil
  Cc: Ken Dreyer, Xuehan Xu, Willem Jan Withagen, Gregory Farnum,
	Nathan Cutler, ceph-maintainers, Alfredo Deza, James Page,
	The Esoteric Order of the Squid Cybernetic

On Wed, Jan 3, 2018 at 10:53 PM, kefu chai <tchaikov@gmail.com> wrote:
> On Wed, Jan 3, 2018 at 1:31 AM, Sage Weil <sweil@redhat.com> wrote:
>> On Tue, 2 Jan 2018, kefu chai wrote:
>>> On Tue, Jan 2, 2018 at 11:28 PM, Ken Dreyer <kdreyer@redhat.com> wrote:
>>> > On Tue, Dec 26, 2017 at 4:24 AM, kefu chai <tchaikov@gmail.com> wrote:
>>> >> right. unless we swing our own homebrew gcc-7.
>>> >
>>> > Can we ask the devtoolset maintainers to change this option?
>>> >
>>> > I imagine they would be interested in this type of feedback from the field.
>>>
>>> https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/2e5ef6c7934d4417e095855478736742b35bd0af/SPECS!gcc.spec#L1026
>>
>> For reference,
>>
>> ```
>>   # Force the old ABI unconditionally, the new one does not work in the
>>   # libstdc++_nonshared.a model against RHEL 6/7 libstdc++.so.6.
>>   sed -i -e 's/\(define[[:blank:]]*_GLIBCXX_USE_DUAL_ABI[[:blank:]]*\)1/\10/' $f
>> ```
>>
>>> I think we need to think about this before sending any inquiries to them.
>>
>> The problem I see is that it is impossible to write/build many native
>> C++11 (or 14 or 17) apps with the toolchain since the old ABI precludes
>> O(1) std::list.
>
> i sort of agree with you. but i think we can still use C++11 (and
> probably 14 or 17)
> with the old ABI. see
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html
>
> <quote>
> the choice of ABI to use is independent of the -std option used to
> compile your code
> </quote>
>
> in other words, if we rely on the O(1) std::list and copy-on-write
> string, then, yes,
> we cannot write/build apps using this toolchain.
>
>>
>> If I'm following the _nonshared thing, it's a feature that lets you build
>> with teh new toolchain but deploy on systems with much older libstdc++
>> installed (new symbols are statically instead of dynamically linked).
>> Is the problem with the "dual ABI" feature?
>
> i think so. also by inspecting
> https://git.centos.org/blob/rpms!devtoolset-7-gcc.git/c7/SOURCES!gcc7-libstdc++-compat.patch,
> libstdc++_nonshared.a includes lots of C++98 and C++11 symbols. in the
> case of list::size(), it is completely implemented in the header file.
> but probably there are also some pieces of libstdc++ implemented in
> the source files. in that case, it would be difficult to include both
> ABIs a class, std::basic_string<> for example, in the same .a file.
> because they are conditionalized by the "_GLIBCXX_USE_CXX11_ABI" macro
> at build time. and they are both in the namespace of std::__cxx11
> namespace.
>
> will drop a mail to SCLs asking for more info on this.


they tried to make it work.

<quote>
The problem is that in the system lib*.so.* + *_nonshared.a model, if some
symbol is exported from the system shared library, we must use that, can't
override that, and the dual ABI support requires several of the system
shared library symbols to behave differently (e.g. for locale facets, need
to register twice as many, one set for old ABI, another for new ABI).
</quote>

so, we need to live with the O(n) std::list::size(), and non
copy-on-write basic_string<> on centos.

-- 
Regards
Kefu Chai

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2018-01-10  9:46 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-10 17:47 C++11, std::list::size(), and trusty Sage Weil
2017-11-13  9:29 ` kefu chai
2017-11-14 12:49   ` kefu chai
     [not found]     ` <CACJqLyamZeEro6htJDV3EDohtH=WbQaHFFXkiRLWiC3t7hWHrA@mail.gmail.com>
     [not found]       ` <CACJqLyZ=Q4rNHZ9y8-DEUdSqCAK8UyjVg9QXYFzxqTf_Rvf-hg@mail.gmail.com>
2017-11-14 13:06         ` kefu chai
2017-11-14 22:04     ` Ken Dreyer
2017-11-15  6:47       ` kefu chai
2017-11-30 10:15         ` kefu chai
2017-11-30 11:00           ` Nathan Cutler
2017-11-30 14:38             ` Sage Weil
2017-12-05 14:53               ` kefu chai
2017-12-05 15:03                 ` Sage Weil
2017-12-05 15:08                   ` kefu chai
2017-12-05 16:40                 ` Adam C. Emerson
2017-12-05 16:51                   ` Jesse Williamson
2017-12-05 16:58                   ` Willem Jan Withagen
2017-12-05 17:51                     ` Gregory Farnum
2017-12-06  9:05                       ` Willem Jan Withagen
2017-12-23  9:46                         ` Xuehan Xu
2017-12-25 14:43                           ` kefu chai
2017-12-25 16:38                             ` kefu chai
2017-12-26  6:35                               ` Xuehan Xu
2017-12-26 11:24                                 ` kefu chai
2017-12-30  3:37                                   ` Xuehan Xu
2018-01-02 15:28                                   ` Ken Dreyer
2018-01-02 15:45                                     ` kefu chai
2018-01-02 17:31                                       ` Sage Weil
2018-01-03 14:53                                         ` kefu chai
2018-01-10  9:46                                           ` kefu chai
2017-12-05 22:23                   ` Nathan Cutler
2017-12-05 22:41                     ` Adam C. Emerson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.