You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Brian Geffon <br...@apache.org> on 2013/03/08 06:13:13 UTC
TS-1742: Freelists: using 64bit versions w/ double word compare and swap
Hello all,
So to those of you familiar with the freelists you know that it works this
way the head pointer uses the upper 16 bits for a version to prevent the
ABA problem. The big drawback to this is that it requires the following
NASTY macros to get at the pointer or the version:
#define FREELIST_POINTER(_x) ((void*)(((((intptr_t)(_x).data)<<16)>>16) | \
(((~((((intptr_t)(_x).data)<<16>>63)-1))>>48)<<48))) // sign extend
#define FREELIST_VERSION(_x) (((intptr_t)(_x).data)>>48)
#define SET_FREELIST_POINTER_VERSION(_x,_p,_v) \
(_x).data = ((((intptr_t)(_p))&0x0000FFFFFFFFFFFFULL) | (((_v)&0xFFFFULL)
<< 48))
Additionally, since this only leaves 16 bits it limits the number of
versions you can have. Well more and more x86_64 processors support DCAS
(double word compare and swap / 128bit CAS). This means that we can use
64bits for a version which basically makes the versions unlimited but more
importantly it takes those macros above and simplifies them to:
#define FREELIST_POINTER(_x) (_x).s.pointer
#define FREELIST_VERSION(_x) (_x).s.version
#define SET_FREELIST_POINTER_VERSION(_x,_p,_v) \
(_x).s.pointer = _p; (_x).s.version = _v
As you can imagine this will have a performance improvement, in my simple
tests I measured a performance improvement of around 6%. As of now I
haven't tried doing this with reclaimable freelists, but if this proves
fruitful maybe I can work with the taobao guys to make sure it wont have
problems with their stuff.
This freelist stuff can be very tricky, so I'll ask anyone who is
interested or understands this code to please review these changes and most
importantly performance test it to make sure this makes. Patch is uploaded
to the ticket: https://issues.apache.org/jira/browse/TS-1742
Thanks
Brian
Re: TS-1742: Freelists: using 64bit versions w/ double word compare and swap
Posted by "Alan M. Carroll" <am...@network-geographics.com>.
It's 8-byte aligned so it's fine in that case. I'll look at that a bit more today.
autoreconf -if
doesn't work any differently than just "-i".
Wednesday, March 13, 2013, 1:45:08 AM, you wrote:
> I dont fully understand how this would work in the 8 byte case if memory
> alignment was the issue.
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Brian Geffon <br...@apache.org>.
I dont fully understand how this would work in the 8 byte case if memory
alignment was the issue.
Brian
On Tue, Mar 12, 2013 at 10:36 PM, Igor Galić <i....@brainsware.org> wrote:
>
>
> ----- Original Message -----
> > Unfortunately this commit breaks my build and executable. FC 18.
> > --enable-debug and --enable-wccp.
> >
> > Building:
> >
> > autoreconf -i
>
> Should you `autoreconf -if` ?
>
> > fails.
> >
> > [amc@yuna ats]$ autoreconf -i
> > configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> > detected in body
> > ../../lib/autoconf/lang.m4:194: AC_LANG_CONFTEST is expanded from...
> > ../../lib/autoconf/general.m4:2730: _AC_RUN_IFELSE is expanded
> > from...
> > ../../lib/m4sugar/m4sh.m4:606: AS_IF is expanded from...
> > ../../lib/autoconf/general.m4:2749: AC_RUN_IFELSE is expanded from...
> > configure.ac:1115: the top level
> > configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> > detected in body
> >
> > When I tried to run it with just a normal rebuild (I tried autoreconf
> > in case there was a configuration problem), I get
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 0x7ffff67fc700 (LWP 14023)]
> > 0x00007ffff7dd5a35 in ink_atomic_cas<__int128> (mem=0x7ffff6d05288,
> > prev=0x00000000000000000000000000000001,
> > next=0x000000000000000000007fffe0015dc1) at ink_atomic.h:153
> > 153 return __sync_bool_compare_and_swap(mem, prev, next);
> > (gdb) up
> > #1 0x00007ffff7dd57b9 in ink_atomiclist_push (l=0x7ffff6d05288,
> > item=0x7fffe0015dc0) at ink_queue.cc:481
> > 481 result = ink_atomic_cas((__int128_t*) & l->head,
> > head.data, item_pair.data);
> > (gdb) up
> > #2 0x00000000006c62e9 in AtomicSLL<UnixNetVConnection,
> > UnixNetVConnection::Link_read_enable_link>::push
> > (this=0x7ffff6d05288, c=0x7fffe0015dc0)
> > at ../../lib/ts/List.h:477
> > 477 void push(C * c) { ink_atomiclist_push(&al, c); }
> > (gdb) up
> > #3 0x00000000006c40ef in UnixNetVConnection::reenable
> > (this=0x7fffe0015dc0,
> > vio=0x7fffe0015ed0) at UnixNetVConnection.cc:721
> > 721 nh->read_enable_list.push(this);
> >
> > I don't see why it segfaults - in the debugger in frame 0 I can look
> > at mem and *mem without a problem. Does the 128 int have to be
> > naturally aligned?
> >
> > Sunday, March 10, 2013, 10:46:53 AM, you wrote:
> >
> > > I'm going to commit this patch today unless there are any last
> > > minute
> > > objections.
> >
> > > Brian
> >
> >
>
> --
> Igor Galić
>
> Tel: +43 (0) 664 886 22 883
> Mail: i.galic@brainsware.org
> URL: http://brainsware.org/
> GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
>
>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Igor Galić <i....@brainsware.org>.
----- Original Message -----
> Unfortunately this commit breaks my build and executable. FC 18.
> --enable-debug and --enable-wccp.
>
> Building:
>
> autoreconf -i
Should you `autoreconf -if` ?
> fails.
>
> [amc@yuna ats]$ autoreconf -i
> configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> detected in body
> ../../lib/autoconf/lang.m4:194: AC_LANG_CONFTEST is expanded from...
> ../../lib/autoconf/general.m4:2730: _AC_RUN_IFELSE is expanded
> from...
> ../../lib/m4sugar/m4sh.m4:606: AS_IF is expanded from...
> ../../lib/autoconf/general.m4:2749: AC_RUN_IFELSE is expanded from...
> configure.ac:1115: the top level
> configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> detected in body
>
> When I tried to run it with just a normal rebuild (I tried autoreconf
> in case there was a configuration problem), I get
>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7ffff67fc700 (LWP 14023)]
> 0x00007ffff7dd5a35 in ink_atomic_cas<__int128> (mem=0x7ffff6d05288,
> prev=0x00000000000000000000000000000001,
> next=0x000000000000000000007fffe0015dc1) at ink_atomic.h:153
> 153 return __sync_bool_compare_and_swap(mem, prev, next);
> (gdb) up
> #1 0x00007ffff7dd57b9 in ink_atomiclist_push (l=0x7ffff6d05288,
> item=0x7fffe0015dc0) at ink_queue.cc:481
> 481 result = ink_atomic_cas((__int128_t*) & l->head,
> head.data, item_pair.data);
> (gdb) up
> #2 0x00000000006c62e9 in AtomicSLL<UnixNetVConnection,
> UnixNetVConnection::Link_read_enable_link>::push
> (this=0x7ffff6d05288, c=0x7fffe0015dc0)
> at ../../lib/ts/List.h:477
> 477 void push(C * c) { ink_atomiclist_push(&al, c); }
> (gdb) up
> #3 0x00000000006c40ef in UnixNetVConnection::reenable
> (this=0x7fffe0015dc0,
> vio=0x7fffe0015ed0) at UnixNetVConnection.cc:721
> 721 nh->read_enable_list.push(this);
>
> I don't see why it segfaults - in the debugger in frame 0 I can look
> at mem and *mem without a problem. Does the 128 int have to be
> naturally aligned?
>
> Sunday, March 10, 2013, 10:46:53 AM, you wrote:
>
> > I'm going to commit this patch today unless there are any last
> > minute
> > objections.
>
> > Brian
>
>
--
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Theo Schlossnagle <je...@omniti.com>.
Hazard pointers are offered by concurrency kit, but can be omitted and in a
normal usage scenario (because everything we use would be leveraged via
inline headers) would never be in trafficserver.
On Mar 8, 2013 3:17 PM, "Brian Geffon" <br...@gmail.com> wrote:
> My only concern with concurrencykit is the use of hazard pointers, it's my
> understanding that hazard pointers are patented by IBM:
>
> http://www.google.com/patents/US20040107227?printsec=description#v=onepage&q&f=false
>
> Brian
>
> On Fri, Mar 8, 2013 at 9:43 AM, Igor Galić <i....@brainsware.org> wrote:
>
> >
> >
> > ----- Original Message -----
> > > On 3/8/13 9:50 AM, Brian Geffon wrote:
> > > > I'm all for this also, i think at minimum we should take advantage
> > > > of
> > > > what we get for free from c++11, especially since we have c++11
> > > > checks
> > > > in our autoconf script already anyway. I had never actually heard
> > > > of
> > > > concurrencykit until now. I know TBB isn't really an option, but
> > > > has
> > > > anyone compared the performance of the data structures of TBB to
> > > > concurrencykit?
> > >
> > > Something to look into. I think can be a solid "goal" for v3.5/3.6,
> > > to clean
> > > up these areas, and perhaps finish all the NUMA stuff as well. I'm
> > > hesitant
> > > to a huge change like this so late in the v3.3/3.4 release cycle
> > > though, but
> > > it's up to the community to decide (of course).
> > > >
> > > > But don't let any of that stop you guys from reviewing this patch
> > > > ;)
> > >
> > > Absolutely. I think this is a good incremental improvement to land
> > > for
> > > v3.3.2, for inclusion with v3.4.
> >
> > +1 on ck in 3.5
> >
> > I'd suggest we wrap up what have into 3.4 and see to stabelizing that
> > If you want to release 3.4.0 in June, we could branch it off in May,
> > to do so, for one, and to allow those who are eager to make fixes
> > to continue working on trunk.
> >
> > > Cheers,
> > >
> > > -- leif
> >
> >
> > -- i
> > Igor Galić
> >
> > Tel: +43 (0) 664 886 22 883
> > Mail: i.galic@brainsware.org
> > URL: http://brainsware.org/
> > GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
> >
> >
>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare and swap
Posted by James Peach <jp...@apache.org>.
On 12/03/2013, at 7:57 PM, Alan M. Carroll <am...@network-geographics.com> wrote:
> Unfortunately this commit breaks my build and executable. FC 18. --enable-debug and --enable-wccp.
>
> Building:
>
> autoreconf -i
>
> fails.
>
> [amc@yuna ats]$ autoreconf -i
> configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call detected in body
> ../../lib/autoconf/lang.m4:194: AC_LANG_CONFTEST is expanded from...
> ../../lib/autoconf/general.m4:2730: _AC_RUN_IFELSE is expanded from...
> ../../lib/m4sugar/m4sh.m4:606: AS_IF is expanded from...
> ../../lib/autoconf/general.m4:2749: AC_RUN_IFELSE is expanded from...
> configure.ac:1115: the top level
> configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call detected in body
>
> When I tried to run it with just a normal rebuild (I tried autoreconf in case there was a configuration problem), I get
>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7ffff67fc700 (LWP 14023)]
> 0x00007ffff7dd5a35 in ink_atomic_cas<__int128> (mem=0x7ffff6d05288,
> prev=0x00000000000000000000000000000001,
> next=0x000000000000000000007fffe0015dc1) at ink_atomic.h:153
> 153 return __sync_bool_compare_and_swap(mem, prev, next);
> (gdb) up
> #1 0x00007ffff7dd57b9 in ink_atomiclist_push (l=0x7ffff6d05288,
> item=0x7fffe0015dc0) at ink_queue.cc:481
> 481 result = ink_atomic_cas((__int128_t*) & l->head, head.data, item_pair.data);
> (gdb) up
> #2 0x00000000006c62e9 in AtomicSLL<UnixNetVConnection, UnixNetVConnection::Link_read_enable_link>::push (this=0x7ffff6d05288, c=0x7fffe0015dc0)
> at ../../lib/ts/List.h:477
> 477 void push(C * c) { ink_atomiclist_push(&al, c); }
> (gdb) up
> #3 0x00000000006c40ef in UnixNetVConnection::reenable (this=0x7fffe0015dc0,
> vio=0x7fffe0015ed0) at UnixNetVConnection.cc:721
> 721 nh->read_enable_list.push(this);
>
> I don't see why it segfaults - in the debugger in frame 0 I can look at mem and *mem without a problem. Does the 128 int have to be naturally aligned?
I checked the Intel manual ... cmpxchg16b needs 16 byte alignment.
>
> Sunday, March 10, 2013, 10:46:53 AM, you wrote:
>
>> I'm going to commit this patch today unless there are any last minute
>> objections.
>
>> Brian
>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare and swap
Posted by "Alan M. Carroll" <am...@network-geographics.com>.
Unfortunately this commit breaks my build and executable. FC 18. --enable-debug and --enable-wccp.
Building:
autoreconf -i
fails.
[amc@yuna ats]$ autoreconf -i
configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call detected in body
../../lib/autoconf/lang.m4:194: AC_LANG_CONFTEST is expanded from...
../../lib/autoconf/general.m4:2730: _AC_RUN_IFELSE is expanded from...
../../lib/m4sugar/m4sh.m4:606: AS_IF is expanded from...
../../lib/autoconf/general.m4:2749: AC_RUN_IFELSE is expanded from...
configure.ac:1115: the top level
configure.ac:1115: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call detected in body
When I tried to run it with just a normal rebuild (I tried autoreconf in case there was a configuration problem), I get
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff67fc700 (LWP 14023)]
0x00007ffff7dd5a35 in ink_atomic_cas<__int128> (mem=0x7ffff6d05288,
prev=0x00000000000000000000000000000001,
next=0x000000000000000000007fffe0015dc1) at ink_atomic.h:153
153 return __sync_bool_compare_and_swap(mem, prev, next);
(gdb) up
#1 0x00007ffff7dd57b9 in ink_atomiclist_push (l=0x7ffff6d05288,
item=0x7fffe0015dc0) at ink_queue.cc:481
481 result = ink_atomic_cas((__int128_t*) & l->head, head.data, item_pair.data);
(gdb) up
#2 0x00000000006c62e9 in AtomicSLL<UnixNetVConnection, UnixNetVConnection::Link_read_enable_link>::push (this=0x7ffff6d05288, c=0x7fffe0015dc0)
at ../../lib/ts/List.h:477
477 void push(C * c) { ink_atomiclist_push(&al, c); }
(gdb) up
#3 0x00000000006c40ef in UnixNetVConnection::reenable (this=0x7fffe0015dc0,
vio=0x7fffe0015ed0) at UnixNetVConnection.cc:721
721 nh->read_enable_list.push(this);
I don't see why it segfaults - in the debugger in frame 0 I can look at mem and *mem without a problem. Does the 128 int have to be naturally aligned?
Sunday, March 10, 2013, 10:46:53 AM, you wrote:
> I'm going to commit this patch today unless there are any last minute
> objections.
> Brian
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Brian Geffon <br...@gmail.com>.
I'm going to commit this patch today unless there are any last minute
objections.
Brian
On Mar 8, 2013, at 2:53 PM, John Plevyak <jp...@acm.org> wrote:
> Since it it is under the simplified BSD, we can simply not include the
> hazard pointer files to avoid any potential problem.
>
> john
>
> On Fri, Mar 8, 2013 at 12:17 PM, Brian Geffon <br...@gmail.com> wrote:
>
>> My only concern with concurrencykit is the use of hazard pointers, it's my
>> understanding that hazard pointers are patented by IBM:
>>
>> http://www.google.com/patents/US20040107227?printsec=description#v=onepage&q&f=false
>>
>> Brian
>>
>> On Fri, Mar 8, 2013 at 9:43 AM, Igor Galić <i....@brainsware.org> wrote:
>>
>>>
>>>
>>> ----- Original Message -----
>>>> On 3/8/13 9:50 AM, Brian Geffon wrote:
>>>>> I'm all for this also, i think at minimum we should take advantage
>>>>> of
>>>>> what we get for free from c++11, especially since we have c++11
>>>>> checks
>>>>> in our autoconf script already anyway. I had never actually heard
>>>>> of
>>>>> concurrencykit until now. I know TBB isn't really an option, but
>>>>> has
>>>>> anyone compared the performance of the data structures of TBB to
>>>>> concurrencykit?
>>>>
>>>> Something to look into. I think can be a solid "goal" for v3.5/3.6,
>>>> to clean
>>>> up these areas, and perhaps finish all the NUMA stuff as well. I'm
>>>> hesitant
>>>> to a huge change like this so late in the v3.3/3.4 release cycle
>>>> though, but
>>>> it's up to the community to decide (of course).
>>>>>
>>>>> But don't let any of that stop you guys from reviewing this patch
>>>>> ;)
>>>>
>>>> Absolutely. I think this is a good incremental improvement to land
>>>> for
>>>> v3.3.2, for inclusion with v3.4.
>>>
>>> +1 on ck in 3.5
>>>
>>> I'd suggest we wrap up what have into 3.4 and see to stabelizing that
>>> If you want to release 3.4.0 in June, we could branch it off in May,
>>> to do so, for one, and to allow those who are eager to make fixes
>>> to continue working on trunk.
>>>
>>>> Cheers,
>>>>
>>>> -- leif
>>>
>>>
>>> -- i
>>> Igor Galić
>>>
>>> Tel: +43 (0) 664 886 22 883
>>> Mail: i.galic@brainsware.org
>>> URL: http://brainsware.org/
>>> GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
>>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by John Plevyak <jp...@acm.org>.
Since it it is under the simplified BSD, we can simply not include the
hazard pointer files to avoid any potential problem.
john
On Fri, Mar 8, 2013 at 12:17 PM, Brian Geffon <br...@gmail.com> wrote:
> My only concern with concurrencykit is the use of hazard pointers, it's my
> understanding that hazard pointers are patented by IBM:
>
> http://www.google.com/patents/US20040107227?printsec=description#v=onepage&q&f=false
>
> Brian
>
> On Fri, Mar 8, 2013 at 9:43 AM, Igor Galić <i....@brainsware.org> wrote:
>
> >
> >
> > ----- Original Message -----
> > > On 3/8/13 9:50 AM, Brian Geffon wrote:
> > > > I'm all for this also, i think at minimum we should take advantage
> > > > of
> > > > what we get for free from c++11, especially since we have c++11
> > > > checks
> > > > in our autoconf script already anyway. I had never actually heard
> > > > of
> > > > concurrencykit until now. I know TBB isn't really an option, but
> > > > has
> > > > anyone compared the performance of the data structures of TBB to
> > > > concurrencykit?
> > >
> > > Something to look into. I think can be a solid "goal" for v3.5/3.6,
> > > to clean
> > > up these areas, and perhaps finish all the NUMA stuff as well. I'm
> > > hesitant
> > > to a huge change like this so late in the v3.3/3.4 release cycle
> > > though, but
> > > it's up to the community to decide (of course).
> > > >
> > > > But don't let any of that stop you guys from reviewing this patch
> > > > ;)
> > >
> > > Absolutely. I think this is a good incremental improvement to land
> > > for
> > > v3.3.2, for inclusion with v3.4.
> >
> > +1 on ck in 3.5
> >
> > I'd suggest we wrap up what have into 3.4 and see to stabelizing that
> > If you want to release 3.4.0 in June, we could branch it off in May,
> > to do so, for one, and to allow those who are eager to make fixes
> > to continue working on trunk.
> >
> > > Cheers,
> > >
> > > -- leif
> >
> >
> > -- i
> > Igor Galić
> >
> > Tel: +43 (0) 664 886 22 883
> > Mail: i.galic@brainsware.org
> > URL: http://brainsware.org/
> > GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
> >
> >
>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Brian Geffon <br...@gmail.com>.
My only concern with concurrencykit is the use of hazard pointers, it's my
understanding that hazard pointers are patented by IBM:
http://www.google.com/patents/US20040107227?printsec=description#v=onepage&q&f=false
Brian
On Fri, Mar 8, 2013 at 9:43 AM, Igor Galić <i....@brainsware.org> wrote:
>
>
> ----- Original Message -----
> > On 3/8/13 9:50 AM, Brian Geffon wrote:
> > > I'm all for this also, i think at minimum we should take advantage
> > > of
> > > what we get for free from c++11, especially since we have c++11
> > > checks
> > > in our autoconf script already anyway. I had never actually heard
> > > of
> > > concurrencykit until now. I know TBB isn't really an option, but
> > > has
> > > anyone compared the performance of the data structures of TBB to
> > > concurrencykit?
> >
> > Something to look into. I think can be a solid "goal" for v3.5/3.6,
> > to clean
> > up these areas, and perhaps finish all the NUMA stuff as well. I'm
> > hesitant
> > to a huge change like this so late in the v3.3/3.4 release cycle
> > though, but
> > it's up to the community to decide (of course).
> > >
> > > But don't let any of that stop you guys from reviewing this patch
> > > ;)
> >
> > Absolutely. I think this is a good incremental improvement to land
> > for
> > v3.3.2, for inclusion with v3.4.
>
> +1 on ck in 3.5
>
> I'd suggest we wrap up what have into 3.4 and see to stabelizing that
> If you want to release 3.4.0 in June, we could branch it off in May,
> to do so, for one, and to allow those who are eager to make fixes
> to continue working on trunk.
>
> > Cheers,
> >
> > -- leif
>
>
> -- i
> Igor Galić
>
> Tel: +43 (0) 664 886 22 883
> Mail: i.galic@brainsware.org
> URL: http://brainsware.org/
> GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
>
>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Igor Galić <i....@brainsware.org>.
----- Original Message -----
> On 3/8/13 9:50 AM, Brian Geffon wrote:
> > I'm all for this also, i think at minimum we should take advantage
> > of
> > what we get for free from c++11, especially since we have c++11
> > checks
> > in our autoconf script already anyway. I had never actually heard
> > of
> > concurrencykit until now. I know TBB isn't really an option, but
> > has
> > anyone compared the performance of the data structures of TBB to
> > concurrencykit?
>
> Something to look into. I think can be a solid "goal" for v3.5/3.6,
> to clean
> up these areas, and perhaps finish all the NUMA stuff as well. I'm
> hesitant
> to a huge change like this so late in the v3.3/3.4 release cycle
> though, but
> it's up to the community to decide (of course).
> >
> > But don't let any of that stop you guys from reviewing this patch
> > ;)
>
> Absolutely. I think this is a good incremental improvement to land
> for
> v3.3.2, for inclusion with v3.4.
+1 on ck in 3.5
I'd suggest we wrap up what have into 3.4 and see to stabelizing that
If you want to release 3.4.0 in June, we could branch it off in May,
to do so, for one, and to allow those who are eager to make fixes
to continue working on trunk.
> Cheers,
>
> -- leif
-- i
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Leif Hedstrom <zw...@apache.org>.
On 3/8/13 9:50 AM, Brian Geffon wrote:
> I'm all for this also, i think at minimum we should take advantage of
> what we get for free from c++11, especially since we have c++11 checks
> in our autoconf script already anyway. I had never actually heard of
> concurrencykit until now. I know TBB isn't really an option, but has
> anyone compared the performance of the data structures of TBB to
> concurrencykit?
Something to look into. I think can be a solid "goal" for v3.5/3.6, to clean
up these areas, and perhaps finish all the NUMA stuff as well. I'm hesitant
to a huge change like this so late in the v3.3/3.4 release cycle though, but
it's up to the community to decide (of course).
>
> But don't let any of that stop you guys from reviewing this patch ;)
Absolutely. I think this is a good incremental improvement to land for
v3.3.2, for inclusion with v3.4.
Cheers,
-- leif
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Brian Geffon <br...@gmail.com>.
I'm all for this also, i think at minimum we should take advantage of
what we get for free from c++11, especially since we have c++11 checks
in our autoconf script already anyway. I had never actually heard of
concurrencykit until now. I know TBB isn't really an option, but has
anyone compared the performance of the data structures of TBB to
concurrencykit?
But don't let any of that stop you guys from reviewing this patch ;)
Brian
On Mar 8, 2013, at 8:18 AM, John Plevyak <jp...@gmail.com> wrote:
> +1
> On Mar 8, 2013 6:39 AM, "Leif Hedstrom" <zw...@apache.org> wrote:
>
>> On 3/8/13 4:24 AM, Theo Schlossnagle wrote:
>>
>>> Would we consider just pulling in concurrencykit? We'll get this and many
>>> other composed primitives from an active community for free.
>>
>> I discussed this with John a while ago, and we both agreed it'd be
>> desirable to use this instead of rolling our own.
>>
>> So +1 from me at least (obviously with careful benchmarking etc.).
>>
>> Also, I'd suggest we do it for 3.5.x, i.e. lets get 3.4 rolled out in our
>> normal release time frame (june). I'm writing up the roadmap for v3.4 as we
>> discussed at ACNA, and will email the mailing list today with the proposal
>> at hand.
>>
>> Cheers,
>>
>>
>> -- Leif
>>
>>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by John Plevyak <jp...@gmail.com>.
+1
On Mar 8, 2013 6:39 AM, "Leif Hedstrom" <zw...@apache.org> wrote:
> On 3/8/13 4:24 AM, Theo Schlossnagle wrote:
>
>> Would we consider just pulling in concurrencykit? We'll get this and many
>> other composed primitives from an active community for free.
>>
>
> I discussed this with John a while ago, and we both agreed it'd be
> desirable to use this instead of rolling our own.
>
> So +1 from me at least (obviously with careful benchmarking etc.).
>
> Also, I'd suggest we do it for 3.5.x, i.e. lets get 3.4 rolled out in our
> normal release time frame (june). I'm writing up the roadmap for v3.4 as we
> discussed at ACNA, and will email the mailing list today with the proposal
> at hand.
>
> Cheers,
>
>
> -- Leif
>
>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Igor Galić <i....@brainsware.org>.
----- Original Message -----
> On 08/03/2013, at 1:10 PM, James Peach <ja...@me.com> wrote:
>
> > On 08/03/2013, at 6:38 AM, Leif Hedstrom <zw...@apache.org> wrote:
> >
> >> On 3/8/13 4:24 AM, Theo Schlossnagle wrote:
> >>> Would we consider just pulling in concurrencykit? We'll get this
> >>> and many
> >>> other composed primitives from an active community for free.
> >>
> >> I discussed this with John a while ago, and we both agreed it'd be
> >> desirable to use this instead of rolling our own.
> >>
> >> So +1 from me at least (obviously with careful benchmarking etc.).
> >
> > CK doesn't support AM; do we care?
>
> *sigh* ARM
patches welcome, I suppose ;)
--
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by James Peach <ja...@me.com>.
On 08/03/2013, at 1:10 PM, James Peach <ja...@me.com> wrote:
> On 08/03/2013, at 6:38 AM, Leif Hedstrom <zw...@apache.org> wrote:
>
>> On 3/8/13 4:24 AM, Theo Schlossnagle wrote:
>>> Would we consider just pulling in concurrencykit? We'll get this and many
>>> other composed primitives from an active community for free.
>>
>> I discussed this with John a while ago, and we both agreed it'd be desirable to use this instead of rolling our own.
>>
>> So +1 from me at least (obviously with careful benchmarking etc.).
>
> CK doesn't support AM; do we care?
*sigh* ARM
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by James Peach <ja...@me.com>.
On 08/03/2013, at 6:38 AM, Leif Hedstrom <zw...@apache.org> wrote:
> On 3/8/13 4:24 AM, Theo Schlossnagle wrote:
>> Would we consider just pulling in concurrencykit? We'll get this and many
>> other composed primitives from an active community for free.
>
> I discussed this with John a while ago, and we both agreed it'd be desirable to use this instead of rolling our own.
>
> So +1 from me at least (obviously with careful benchmarking etc.).
CK doesn't support AM; do we care?
J
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Leif Hedstrom <zw...@apache.org>.
On 3/8/13 4:24 AM, Theo Schlossnagle wrote:
> Would we consider just pulling in concurrencykit? We'll get this and many
> other composed primitives from an active community for free.
I discussed this with John a while ago, and we both agreed it'd be desirable
to use this instead of rolling our own.
So +1 from me at least (obviously with careful benchmarking etc.).
Also, I'd suggest we do it for 3.5.x, i.e. lets get 3.4 rolled out in our
normal release time frame (june). I'm writing up the roadmap for v3.4 as we
discussed at ACNA, and will email the mailing list today with the proposal
at hand.
Cheers,
-- Leif
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by Theo Schlossnagle <je...@omniti.com>.
Would we consider just pulling in concurrencykit? We'll get this and many
other composed primitives from an active community for free.
On Mar 8, 2013 12:49 AM, "John Plevyak" <jp...@gmail.com> wrote:
> I'll take a look. I was thinking we should make this move. Glad to see
> it.
> On Mar 7, 2013 9:13 PM, "Brian Geffon" <br...@apache.org> wrote:
>
> > Hello all,
> >
> > So to those of you familiar with the freelists you know that it works
> this
> > way the head pointer uses the upper 16 bits for a version to prevent the
> > ABA problem. The big drawback to this is that it requires the following
> > NASTY macros to get at the pointer or the version:
> >
> > #define FREELIST_POINTER(_x) ((void*)(((((intptr_t)(_x).data)<<16)>>16)
> | \
> > (((~((((intptr_t)(_x).data)<<16>>63)-1))>>48)<<48))) // sign extend
> > #define FREELIST_VERSION(_x) (((intptr_t)(_x).data)>>48)
> > #define SET_FREELIST_POINTER_VERSION(_x,_p,_v) \
> > (_x).data = ((((intptr_t)(_p))&0x0000FFFFFFFFFFFFULL) | (((_v)&0xFFFFULL)
> > << 48))
> >
> > Additionally, since this only leaves 16 bits it limits the number of
> > versions you can have. Well more and more x86_64 processors support DCAS
> > (double word compare and swap / 128bit CAS). This means that we can use
> > 64bits for a version which basically makes the versions unlimited but
> more
> > importantly it takes those macros above and simplifies them to:
> >
> > #define FREELIST_POINTER(_x) (_x).s.pointer
> > #define FREELIST_VERSION(_x) (_x).s.version
> > #define SET_FREELIST_POINTER_VERSION(_x,_p,_v) \
> > (_x).s.pointer = _p; (_x).s.version = _v
> >
> > As you can imagine this will have a performance improvement, in my simple
> > tests I measured a performance improvement of around 6%. As of now I
> > haven't tried doing this with reclaimable freelists, but if this proves
> > fruitful maybe I can work with the taobao guys to make sure it wont have
> > problems with their stuff.
> >
> > This freelist stuff can be very tricky, so I'll ask anyone who is
> > interested or understands this code to please review these changes and
> most
> > importantly performance test it to make sure this makes. Patch is
> uploaded
> > to the ticket: https://issues.apache.org/jira/browse/TS-1742
> >
> > Thanks
> > Brian
> >
>
Re: TS-1742: Freelists: using 64bit versions w/ double word compare
and swap
Posted by John Plevyak <jp...@gmail.com>.
I'll take a look. I was thinking we should make this move. Glad to see
it.
On Mar 7, 2013 9:13 PM, "Brian Geffon" <br...@apache.org> wrote:
> Hello all,
>
> So to those of you familiar with the freelists you know that it works this
> way the head pointer uses the upper 16 bits for a version to prevent the
> ABA problem. The big drawback to this is that it requires the following
> NASTY macros to get at the pointer or the version:
>
> #define FREELIST_POINTER(_x) ((void*)(((((intptr_t)(_x).data)<<16)>>16) | \
> (((~((((intptr_t)(_x).data)<<16>>63)-1))>>48)<<48))) // sign extend
> #define FREELIST_VERSION(_x) (((intptr_t)(_x).data)>>48)
> #define SET_FREELIST_POINTER_VERSION(_x,_p,_v) \
> (_x).data = ((((intptr_t)(_p))&0x0000FFFFFFFFFFFFULL) | (((_v)&0xFFFFULL)
> << 48))
>
> Additionally, since this only leaves 16 bits it limits the number of
> versions you can have. Well more and more x86_64 processors support DCAS
> (double word compare and swap / 128bit CAS). This means that we can use
> 64bits for a version which basically makes the versions unlimited but more
> importantly it takes those macros above and simplifies them to:
>
> #define FREELIST_POINTER(_x) (_x).s.pointer
> #define FREELIST_VERSION(_x) (_x).s.version
> #define SET_FREELIST_POINTER_VERSION(_x,_p,_v) \
> (_x).s.pointer = _p; (_x).s.version = _v
>
> As you can imagine this will have a performance improvement, in my simple
> tests I measured a performance improvement of around 6%. As of now I
> haven't tried doing this with reclaimable freelists, but if this proves
> fruitful maybe I can work with the taobao guys to make sure it wont have
> problems with their stuff.
>
> This freelist stuff can be very tricky, so I'll ask anyone who is
> interested or understands this code to please review these changes and most
> importantly performance test it to make sure this makes. Patch is uploaded
> to the ticket: https://issues.apache.org/jira/browse/TS-1742
>
> Thanks
> Brian
>