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),
On 2015/08/24 13:43:12, Michael Lippautz wrote:
add
   free_list_mutex_(),

Why? The default constructor is called in this case anyway.

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_);
On 2015/08/24 13:43:12, Michael Lippautz wrote:
Please add a block to make clear which parts are to-be-locked.

I would have just locked the rest of the function, but we can do better.
Here is your scope. Done.

https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode345
src/heap/spaces.cc:345: LOG(isolate_, DeleteEvent("InitialChunk",
addr));
On 2015/08/24 13:43:12, Michael Lippautz wrote:
"InitialChunk"?

This was the original string. We can change it to NewSpace.

https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode348
src/heap/spaces.cc:348: size_t size = reservation->size();
On 2015/08/24 13:43:12, Michael Lippautz wrote:
const size_t size = ..

Done.

https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode747
src/heap/spaces.cc:747: void MemoryAllocator::PreFreeMemory(MemoryChunk*
chunk) {
On 2015/08/24 13:43:12, Michael Lippautz wrote:
Should we add a flag to {MemoryChunk} indicating that it has been
PreFreed()?
(The flag could be checked in PerformFree as DCHECK.)

Good idea. Done.

https://codereview.chromium.org/1306183003/diff/60001/src/heap/spaces.cc#newcode776
src/heap/spaces.cc:776: void
MemoryAllocator::PerformFreeMemory(MemoryChunk* chunk) {
On 2015/08/24 13:43:12, Michael Lippautz wrote:
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.)

As discussed offline, this method should not be static since it calls
deeper into the memory allocator. However, it makes sense to perform the
deallocation of the previously alloced data structures in a method on
the memory chunk.

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.

Reply via email to