Thanks for doing this cleanup! It is going in the right direction. I have a
couple of comments...
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces-inl.h
File src/heap/spaces-inl.h (right):
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces-inl.h#newcode348
src/heap/spaces-inl.h:348: int filler_size =
Heap::GetFillToAlign(allocation_info_.top(), alignment);
Keep top in a temporary variable.
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces-inl.h#newcode354
src/heap/spaces-inl.h:354: if (EnsureAllocation(size_in_bytes,
alignment) == false) {
Instead of "== false" use "!".
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces-inl.h#newcode357
src/heap/spaces-inl.h:357:
if top changed, read out top again and recalculate
aligned_size_in_bytes.
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces-inl.h#newcode359
src/heap/spaces-inl.h:359: aligned_size_in_bytes) {
This check should not be needed.
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces-inl.h#newcode365
src/heap/spaces-inl.h:365: HeapObject* obj =
HeapObject::FromAddress(allocation_info_.top());
Use the temporary top variable here.
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces-inl.h#newcode379
src/heap/spaces-inl.h:379: AllocationResult
NewSpace::AllocateRawUnaligned(int size_in_bytes) {
Comments from above apply also here.
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces.cc#newcode1474
src/heap/spaces.cc:1474: if (allocation_info_.limit() < high) {
This method should in principle take care of two cases:
1) there is room in this page: top + size_in_bytes < high
2) there is no room in this page.
Case one right now does not guarantee that there is room. We just go
there whenever limit is smaller than high, but a huge allocation would
just go over high.
We should restructure the code to check if there is room: top +
size_in_bytes < high. If there is room perform case 1) If there is no
room add a fresh page (if possible) and do the special accounting. Then
perform case 1).
This method should return true if it can guarantee that the bump pointer
allocation will succeed.
https://codereview.chromium.org/1265443003/diff/1/src/heap/spaces.cc#newcode1495
src/heap/spaces.cc:1495: } else {
Remove the else case and just return false.
https://codereview.chromium.org/1265443003/
--
--
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.
