Mostly nits and food for thought.
https://codereview.chromium.org/1306183003/diff/60001/src/heap/heap.cc
File src/heap/heap.cc (right):
https://codereview.chromium.org/1306183003/diff/60001/src/heap/heap.cc#newcode6583
src/heap/heap.cc:6583: void Heap::FreeQueuedChunks(MemoryChunk*
list_head) {
This is a candidate for a static on {MemoryChunk}?
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc
File src/heap/spaces.cc (right):
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode80
src/heap/spaces.cc:80: code_range_(NULL),
add
free_list_mutex_(),
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode166
src/heap/spaces.cc:166: base::LockGuard<base::Mutex>
free_list_lock_guard(&free_list_mutex_);
Please add a block to make clear which parts are to-be-locked.
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode345
src/heap/spaces.cc:345: LOG(isolate_, DeleteEvent("InitialChunk",
addr));
"InitialChunk"?
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode348
src/heap/spaces.cc:348: size_t size = reservation->size();
const size_t size = ..
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode747
src/heap/spaces.cc:747: void MemoryAllocator::PreFreeMemory(MemoryChunk*
chunk) {
Should we add a flag to {MemoryChunk} indicating that it has been
PreFreed()? (The flag could be checked in PerformFree as DCHECK.)
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode776
src/heap/spaces.cc:776: void
MemoryAllocator::PerformFreeMemory(MemoryChunk* chunk) {
We could make this a static on {MemoryChunk}.
{MemoryChunk} is sort of statically created (just cast to a MemoryChunk
after allocating it, so it would probably make sense to destroy it the
same way.)
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.h
File src/heap/spaces.h (right):
https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.h#newcode1088
src/heap/spaces.h:1088: // bookkeeping and calls the allocation
callback.
Can we extend this comment to clarify that this code is meant to be run
on the main thread?
https://codereview.chromium.org/1306183003/
--
--
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.
