I like the new FreeListCategory class much more now. Just a few nits. The complexity of the two concurrent operation is pretty high though.
https://codereview.chromium.org/11348174/diff/9/src/spaces.cc File src/spaces.cc (right): https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1940 src/spaces.cc:1940: ConcatenateListsConcurrentAdd(FreeListCategory* category) { Let's break the line after the opening parenthesis. http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Declarations_and_Definitions https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1951 src/spaces.cc:1951: } The adjustment category_end->next to current_top is missing. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1965 src/spaces.cc:1965: ConcatenateListsConcurrentRemove(FreeListCategory* category) { Likewise. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1978 src/spaces.cc:1978: category_top = category->top(); This requires the memory model to ensure that we don't reorder load after load. This doesn't hold on ARM ... so I think we need a memory barrier after reading category->top. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1988 src/spaces.cc:1988: reinterpret_cast<AtomicWord>(category_end), 0); Since we don't need the category->end to be in a certain state once our commit ran through, we should just ignore it completely and let the producer fix it. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode2024 src/spaces.cc:2024: } We need to adjust the end pointer in case you evicted the last elements (or completely cleared the free list). https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode2162 src/spaces.cc:2162: *cur = node->next(); We also need to reset the end pointer in case we picked the last entry. https://codereview.chromium.org/11348174/diff/9/src/spaces.h File src/spaces.h (right): https://codereview.chromium.org/11348174/diff/9/src/spaces.h#newcode1428 src/spaces.h:1428: FreeListNode* top() const { Let's compact the simple accessors a little by using just one line to implement them and possibly group those that work on the same variable together (by removing the empty newline between them). https://codereview.chromium.org/11348174/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
