[Review] Review results for Lockfree
All, The review for the Lockfree library written by Tim Blechman started July 18th 2011 and ended on July 28th. I counted 8 votes, none of which were NO. All authors votes YES, some made additional requests. Overall, the verdict of the community was clear: Tim Blechmann's Lockfree library is ACCEPTED The discussion was lively and it touched on several points. Design ------ The design of the library is sound, the API is usable. Except for naming issues (discussed below) almost no comments have been made on how to change it. People reviewing the library would like to see a more modular approach to lockfree data structures in general, possibly by exposing building blocks or by utilizing policies. The general interest was to have a more diverse set of data structures available, such as a lock-free linked list or bounded and fixed-sized data structures. Atomics Library --------------- As the initial review announcement stated, Lockfree depends on an external Atomics library, which has to be separately reviewed in order to get into Boost as a first class library. There has been some discussions whether Lockfree could be accepted without Atomics being reviewed. Others suggested Lockfree may be reviewed and added to Boost SVN only after Atomics got reviewed (this was mentioned in the review announcement as well). After all those discussions and based on the wide interest Lockfree data structures have, I'd suggest to add the current Atomics library as an implementation detail to Lockfree. Special handling of compilers which already have implemented std::atomics would be good if added to Lockfree, though. Naming ------ The consensus was that the naming of the reviewed data structures has to be changed. The names should be either fifo and lifo or queue and stack. AFAIK, Tim already addresses this point. Documentation ------------- The consensus of almost everybody referring to the documentation was that it needs more work. Here is a short (but non-exhaustive) list of things being mentioned: - It lacks rationale and information about the implementation - The class synopsis of the data structures should be accessible from the "Reference" page. - Make non-thread-safe parts more explicit (fifo::empty is described as non-thread-safe) - Document exception guarantees - More information needed on internals, the design, and rationale I would like to thank all who participated in the discussions. Regards Hartmut --------------- http://boost-spirit.com
The review for the Lockfree library written by Tim Blechman started July 18th 2011 and ended on July 28th. I counted 8 votes, none of which were NO. All authors votes YES, some made additional requests. Overall, the verdict of the community was clear:
Tim Blechmann's Lockfree library is ACCEPTED
i will try to address the comments in the next few weeks! considering the deadline for new libraries is in two weeks, i'd probably wait with merging boost.lockfree until 1.49. thanks a lot to hartmut for managing the review and to all the reviewers! cheers, tim
On 8/23/2011 at 3:13 AM, in message <01c601cc6131$dc5d5e20$95181a60$@gmail.com>, "Hartmut Kaiser" <hartmut.kaiser@gmail.com> wrote: All,
The review for the Lockfree library written by Tim Blechman started July 18th 2011 and ended on July 28th. I counted 8 votes, none of which were NO. All authors votes YES, some made additional requests. Overall, the verdict of the community was clear:
Tim Blechmann's Lockfree library is ACCEPTED
The discussion was lively and it touched on several points.
Design ------ The design of the library is sound, the API is usable. Except for naming issues (discussed below) almost no comments have been made on how to change it.
People reviewing the library would like to see a more modular approach to lockfree data structures in general, possibly by exposing building blocks or by utilizing policies. The general interest was to have a more diverse set of data structures available, such as a lock-free linked list or bounded and fixed-sized data structures.
Atomics Library --------------- As the initial review announcement stated, Lockfree depends on an external Atomics library, which has to be separately reviewed in order to get into Boost as a first class library. There has been some discussions whether Lockfree could be accepted without Atomics being reviewed. Others suggested Lockfree may be reviewed and added to Boost SVN only after Atomics got reviewed (this was mentioned in the review announcement as well).
After all those discussions and based on the wide interest Lockfree data structures have, I'd suggest to add the current Atomics library as an implementation detail to Lockfree. Special handling of compilers which already have implemented std::atomics would be good if added to Lockfree, though.
Naming ------ The consensus was that the naming of the reviewed data structures has to be changed. The names should be either fifo and lifo or queue and stack. AFAIK, Tim already addresses this point.
Documentation ------------- The consensus of almost everybody referring to the documentation was that it needs more work. Here is a short (but non-exhaustive) list of things being mentioned: - It lacks rationale and information about the implementation - The class synopsis of the data structures should be accessible from the "Reference" page. - Make non-thread-safe parts more explicit (fifo::empty is described as non-thread-safe) - Document exception guarantees - More information needed on internals, the design, and rationale
I would like to thank all who participated in the discussions.
Regards Hartmut --------------- http://boost-spirit.com
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hartmut, just wanted to say thanks for managing the review. Unfortunately I wasn't able to submit a review, but I was following the review process with some interest. Well done Tim, I look forward to trying out your library :-) Cheers, Ralf -- This message is subject to the CSIR's copyright terms and conditions, e-mail legal notice, and implemented Open Document Format (ODF) standard. The full disclaimer details can be found at http://www.csir.co.za/disclaimer.html. This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
Hi Hartmut, Hartmut Kaiser wrote:
Atomics Library --------------- As the initial review announcement stated, Lockfree depends on an external Atomics library, which has to be separately reviewed in order to get into Boost as a first class library. There has been some discussions whether Lockfree could be accepted without Atomics being reviewed. Others suggested Lockfree may be reviewed and added to Boost SVN only after Atomics got reviewed (this was mentioned in the review announcement as well).
After all those discussions and based on the wide interest Lockfree data structures have, I'd suggest to add the current Atomics library as an implementation detail to Lockfree.
I didn't review the Atomics code because Tim said he wanted to wait for it to be reviewed and accepted separately. Did anyone else look at it? If this were to change and it were to be included in Lockfree as an implementation detail, I feel that a mini-review of the Atomics code would be worthwhile. Regards, Phil.
hi,
Atomics Library --------------- As the initial review announcement stated, Lockfree depends on an external Atomics library, which has to be separately reviewed in order to get into Boost as a first class library. There has been some discussions whether Lockfree could be accepted without Atomics being reviewed. Others suggested Lockfree may be reviewed and added to Boost SVN only after Atomics got reviewed (this was mentioned in the review announcement as well).
After all those discussions and based on the wide interest Lockfree data structures have, I'd suggest to add the current Atomics library as an implementation detail to Lockfree.
I didn't review the Atomics code because Tim said he wanted to wait for it to be reviewed and accepted separately. Did anyone else look at it? If this were to change and it were to be included in Lockfree as an implementation detail, I feel that a mini-review of the Atomics code would be worthwhile.
i am somehow familiar with boost.atomic and reviewed about 80% of the code when using it for boost.lockfree. since boost.atomic implements a c++11 feature, i think it should be sufficient to have a mini-review and would be willing to manage the review. cheers, tim
On 22/08/2011 6:13 PM, Hartmut Kaiser wrote:
The general interest was to have a more diverse set of data structures available, such as a lock-free linked list or bounded and fixed-sized data structures.
I've been sitting on a lock-free doubly-linked list, perhaps there's interest in porting it to boost. It's ABA problem free using only a 32-bit CAS. It's built off of a lock-free memory manager that could serve as the foundation for other container types (map, unordered map, bag). The doubly-linked list, algorithm supports parallelism for disjoint accesses so contention can decrease as the list size increases: https://github.com/Kometes/Honeycomb/blob/master/Honeycomb/Src/Common/Inc/Th... The memory manager, expands to meet demand and recycles its nodes: https://github.com/Kometes/Honeycomb/blob/master/Honeycomb/Src/Common/Inc/Th... Test code: https://github.com/Kometes/Honeycomb/blob/master/Editor/Src/Win/Src/Misc/Tes... It's tested on x86 compiling with VS2010/11.
hi dan,
The general interest was to have a more diverse set of data structures available, such as a lock-free linked list or bounded and fixed-sized data structures.
I've been sitting on a lock-free doubly-linked list, perhaps there's interest in porting it to boost. It's ABA problem free using only a 32-bit CAS. It's built off of a lock-free memory manager that could serve as the foundation for other container types (map, unordered map, bag).
The doubly-linked list, algorithm supports parallelism for disjoint accesses so contention can decrease as the list size increases:
i currently wouldn't have any time to port it to boost.lockfree (especially since it seems to use rather different concepts than the ones that i have implemented for now it would probably need some adaptions) ... but i will try to assist you, if you want to try to integrate it into boost.lockfree. iac, maybe i can find the time to read the specific papers in the next few days ... cheers, tim
Hi Tim, unfortunately I don't have time to port it either, but if someone is interested it should be a straight-forward port provided that c++11-style atomics and thread-local variables are available. I should note that in my lib all atomic loads/stores are relaxed by default (rather than seq_cst). On 07/10/2011 4:12 AM, Tim Blechmann wrote:
hi dan,
The general interest was to have a more diverse set of data structures available, such as a lock-free linked list or bounded and fixed-sized data structures. I've been sitting on a lock-free doubly-linked list, perhaps there's interest in porting it to boost. It's ABA problem free using only a 32-bit CAS. It's built off of a lock-free memory manager that could serve as the foundation for other container types (map, unordered map, bag).
The doubly-linked list, algorithm supports parallelism for disjoint accesses so contention can decrease as the list size increases: i currently wouldn't have any time to port it to boost.lockfree (especially since it seems to use rather different concepts than the ones that i have implemented for now it would probably need some adaptions) ... but i will try to assist you, if you want to try to integrate it into boost.lockfree.
iac, maybe i can find the time to read the specific papers in the next few days ...
cheers, tim
Hi Tim, unfortunately I don't have time to port it either, but if someone is interested it should be a straight-forward port provided that c++11-style atomics and thread-local variables are available. I should note that in my lib all atomic loads/stores are relaxed by default (rather than seq_cst).
porting may be trivial, but integrating it into boost.lockfree to match class and policy interface and documenting it in a rock-solid way is not as trivial anymore tim
The general interest was to have a more diverse set of data structures available, such as a lock-free linked list or bounded and fixed-sized data structures.
I've been sitting on a lock-free doubly-linked list, perhaps there's interest in porting it to boost. It's ABA problem free using only a 32-bit CAS. It's built off of a lock-free memory manager that could serve as the foundation for other container types (map, unordered map, bag).
The doubly-linked list, algorithm supports parallelism for disjoint
accesses so contention can decrease as the list size increases: i currently wouldn't have any time to port it to boost.lockfree (especially since it seems to use rather different concepts than the ones that i have implemented for now it would probably need some adaptions) ... but i will try to assist you, if you want to try to integrate it into boost.lockfree.
iac, maybe i can find the time to read the specific papers in the next few days ...
cheers, tim
participants (5)
-
Dan Carter -
Hartmut Kaiser -
Phil Endecott -
Ralf Globisch -
Tim Blechmann