wrong branch.

LGTM (though I am bit confused by some seemingly unrelated changes here and
there)


http://codereview.chromium.org/5826004/diff/1/src/assembler.h
File src/assembler.h (right):

http://codereview.chromium.org/5826004/diff/1/src/assembler.h#newcode537
src/assembler.h:537: // Write barrier.
barrier -> buffer.

http://codereview.chromium.org/5826004/diff/1/src/write-buffer.cc
File src/write-buffer.cc (right):

http://codereview.chromium.org/5826004/diff/1/src/write-buffer.cc#newcode39
src/write-buffer.cc:39: virtual_memory_ = new
VirtualMemory(kWriteBufferSize * 3);
This allocation looks OK for single buffer but I am not sure that it is
OK for multiple buffers (if we decide to have many of them).

On Windows which has allocation granularity 64k this is going to waste
98k of address space.

Even on Linux it might be beneficial to return unused parts of this
memory back to OS.

I see no obvious benefit for single bit vs. several bits on x86 (though
benefit is obvious on ARM): mask = (kWriteBufferOverflowBit - 1),
alignent = kWriteBufferOverflowBit.

http://codereview.chromium.org/5826004/diff/1/src/write-buffer.h
File src/write-buffer.h (right):

http://codereview.chromium.org/5826004/diff/1/src/write-buffer.h#newcode39
src/write-buffer.h:39: class WriteBuffer : public AllStatic {
Consider adding a small comment about the role of this class.

http://codereview.chromium.org/5826004/diff/1/src/write-buffer.h#newcode47
src/write-buffer.h:47: static const int kWriteBufferOverflowBit = 1 <<
15;
15 and 32 are connected.

consider expressing this connection through introduction of a constant

http://codereview.chromium.org/5826004/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to