https://codereview.chromium.org/391413006/diff/40001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/391413006/diff/40001/src/heap.cc#newcode5974
src/heap.cc:5974: GCTracer::Event::Event()
On 2014/07/22 15:21:30, Hannes Payer wrote:
Why do we need the default constructor?

RingBuffer allocates an array of Events. The default constructor is
required for that. We can leave the Events uninitialized, though.

https://codereview.chromium.org/391413006/diff/40001/src/heap.cc#newcode6059
src/heap.cc:6059: if (collector == SCAVENGER)
On 2014/07/22 15:21:30, Hannes Payer wrote:
coding style: if/else likes curly braces

Done.

https://codereview.chromium.org/391413006/diff/40001/src/heap.cc#newcode6088
src/heap.cc:6088: scavenger_events_.push_back(pending_);
On 2014/07/22 15:21:30, Hannes Payer wrote:
coding style: if/else likes curly braces

Done.

https://codereview.chromium.org/391413006/diff/40001/src/heap.cc#newcode6130
src/heap.cc:6130: mark_compactor_events_.back()->end_time)
On 2014/07/22 15:21:30, Hannes Payer wrote:
coding style: if/else likes curly braces

Done.

https://codereview.chromium.org/391413006/diff/40001/src/heap.cc#newcode6159
src/heap.cc:6159: return last_in_other_buffer;
On 2014/07/22 15:21:30, Hannes Payer wrote:
coding style: if/else likes curly braces

Done.

https://codereview.chromium.org/391413006/diff/40001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/391413006/diff/40001/src/heap.h#newcode607
src/heap.h:607: end_ = (end_ + 1) % (MAX_SIZE + 1);
On 2014/07/22 15:21:31, Hannes Payer wrote:
Hmm, MAX_SIZE+1 makes the code really complicated. And, the anchor
elements is
just there until it gets overwritten (logically it is still there
after that). I
am wondering if it wouldn't be cleaner if we would handle the case
where there
is no previous element externally... WDYT?

That is unrelated to anchor elements. The extra element is required to
correctly handle wrap around cases in the ring buffer. Without the extra
element, we couldn't distinguish an empty ring buffer from one that is
completely filled (begin_ == end_ in both cases) and the iterator
pointing to the first element would be identical to the end() iterator.

https://codereview.chromium.org/391413006/diff/40001/src/heap.h#newcode615
src/heap.h:615: size_t end_;
On 2014/07/22 15:21:31, Hannes Payer wrote:
DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/391413006/diff/40001/src/heap.h#newcode676
src/heap.h:676: INVALID = 3
On 2014/07/22 15:21:30, Hannes Payer wrote:
what is the purpose of invalid?

Not really required. See commend on default constructor of Event.

https://codereview.chromium.org/391413006/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to