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) { On 2012/11/29 14:31:20, Michael Starzinger wrote:
Let's break the line after the opening parenthesis.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Declarations_and_Definitions Done. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1951 src/spaces.cc:1951: } On 2012/11/29 14:31:20, Michael Starzinger wrote:
The adjustment category_end->next to current_top is missing.
Done. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1965 src/spaces.cc:1965: ConcatenateListsConcurrentRemove(FreeListCategory* category) { On 2012/11/29 14:31:20, Michael Starzinger wrote:
Likewise.
Done. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1978 src/spaces.cc:1978: category_top = category->top(); On 2012/11/29 14:31:20, Michael Starzinger wrote:
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.
Done. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode1988 src/spaces.cc:1988: reinterpret_cast<AtomicWord>(category_end), 0); On 2012/11/29 14:31:20, Michael Starzinger wrote:
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. Done. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode2024 src/spaces.cc:2024: } On 2012/11/29 14:31:20, Michael Starzinger wrote:
We need to adjust the end pointer in case you evicted the last
elements (or
completely cleared the free list).
Done. https://codereview.chromium.org/11348174/diff/9/src/spaces.cc#newcode2162 src/spaces.cc:2162: *cur = node->next(); On 2012/11/29 14:31:20, Michael Starzinger wrote:
We also need to reset the end pointer in case we picked the last
entry. Done. 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 { On 2012/11/29 14:31:20, Michael Starzinger wrote:
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).
Done. https://codereview.chromium.org/11348174/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
