Thanks Kevin and Vitaly!
http://codereview.chromium.org/267077/diff/2001/22 File src/global-handles.h (right): http://codereview.chromium.org/267077/diff/2001/22#newcode57 Line 57: typedef void (*WeakReferenceVisitor)(Object* object, void* parameter); On 2009/10/14 15:11:31, Kevin Millikin wrote: > This shouldn't be called ...Visitor unless it's a subclass of the ObjectVisitor. > I can't think of a better name, though. Renamed to WeakReferenceGuest. http://codereview.chromium.org/267077/diff/2001/23 File src/heap-profiler.cc (right): http://codereview.chromium.org/267077/diff/2001/23#newcode587 Line 587: object.ClearWeak(); On 2009/10/14 17:05:55, Vitaly wrote: > You probably want to Dispose() the weak handle here. ClearWeak() turns it into a > strong handle. Yes, I want the handle to disappear. Thanks for spotting this! http://codereview.chromium.org/267077/diff/2001/23#newcode597 Line 597: static_cast<Address*>(trace))); On 2009/10/14 15:11:31, Kevin Millikin wrote: > Can reinterpret_cast here. Done. http://codereview.chromium.org/267077/diff/2001/23#newcode643 Line 643: GlobalHandles::IterateWeakRoots(PrintProducerStackTrace, StackWeakReferenceCallback); On 2009/10/14 17:05:55, Vitaly wrote: > Line too long? Fixed. I forgot run presubmit. http://codereview.chromium.org/267077/diff/2001/23#newcode665 Line 665: HandleScope scope; On 2009/10/14 17:05:55, Vitaly wrote: > Persistent handles don't need handle scope. This remained from my attempt to use public API that required lots of handles to be created. Removed. http://codereview.chromium.org/267077/diff/2001/23#newcode666 Line 666: Handle<Object> handle = GlobalHandles::Create(obj); On 2009/10/14 17:05:55, Vitaly wrote: > Why not use include/v8.h API here? I tried initially, but this caused me to write ugly redundant chains of handles conversions back and forth, so I decided to boil it down by using internal API directly. http://codereview.chromium.org/267077/diff/2001/24 File src/heap-profiler.h (right): http://codereview.chromium.org/267077/diff/2001/24#newcode261 Line 261: static bool can_log; On 2009/10/14 15:11:31, Kevin Millikin wrote: > can_log_. I'd prefer that it was private, and there was a Setup function that > set it to true. Done. http://codereview.chromium.org/267077/diff/2001/25 File src/heap.cc (right): http://codereview.chromium.org/267077/diff/2001/25#newcode3313 Line 3313: ProducerHeapProfile::can_log = true; On 2009/10/14 15:11:31, Kevin Millikin wrote: > If you have a ProducerHeapProfile::Setup function you can call it here (with a > comment that it should be called last during heap setup). Done. http://codereview.chromium.org/267077 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
