LGTM with comments.

http://codereview.chromium.org/7621014/diff/1/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/7621014/diff/1/src/heap.cc#newcode733
src/heap.cc:733: new_space()->Capacity() +
AllocationFactor(old_gen_size));
maybe instead of XFactor you should call it XDelta. XIncrease, XGrowth,
etc.

http://codereview.chromium.org/7621014/diff/1/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/7621014/diff/1/src/heap.h#newcode1216
src/heap.h:1216: static inline intptr_t PromotionFactor(intptr_t size) {
More comments about this magical functions would be appreciated.

http://codereview.chromium.org/7621014/diff/1/src/heap.h#newcode1226
src/heap.h:1226: PromotionFactor(bytes) * old_gen_limit_factor_,
multiplying factor by factor. confusing. more comments please.

old_gen_limit_factor_ is confusing name.

http://codereview.chromium.org/7621014/diff/1/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/7621014/diff/1/src/spaces.cc#newcode1933
src/spaces.cc:1933: if (FLAG_trace_gc && first_unswept_page_ != NULL) {
Ditto

http://codereview.chromium.org/7621014/diff/1/src/spaces.h
File src/spaces.h (right):

http://codereview.chromium.org/7621014/diff/1/src/spaces.h#newcode1448
src/spaces.h:1448: if (FLAG_trace_gc) {
I think it should be under verbose flag.

I prefer to keep trace_gc output clean and simple.

http://codereview.chromium.org/7621014/

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

Reply via email to