A couple of public service announcements: V8 development is done against the bleeding_edge branch ( v8.googlecode.com/svn/branches/bleeding_edge). Since patches will not be committed directly to trunk, they should not be reviewed against trunk either. Trunk can lag bleeding_edge by up to a week.
Something is wrong with your svn checkout or gcl configuration. The file svn/codereview.settings is being ignored and so uploaded issues are not cc'd to the v8-dev mailing list. regards, Kevin On Thu, Sep 2, 2010 at 9:44 AM, <[email protected]> wrote: > > http://codereview.chromium.org/3329006/diff/2001/3001 > File include/v8.h (right): > > http://codereview.chromium.org/3329006/diff/2001/3001#newcode2362 > include/v8.h:2362: typedef void (*ExecutableAllocCallback)(int > size_executable); > I think this callback type is to specific in just reporting executable > memory. How about having > > MemoryAllocationCallback(AllocationSpace space, MemoryType type, > AllocationAction action, int size) > > with enums > > enum AllocationSpace { > kAllocationSpaceYoung, > kAllocationSpacePaged, > kAllocationSpaceLarge > } > > enum MemoryType { > kMemoryTypeData, > kMemoryTypeExecutable > } > > enum AllocationAction { > kAllocationActionAllocate, > kAllocationActionFree > } > > Something like GCPrologueCallback and GCEpilogueCallback. > > http://codereview.chromium.org/3329006/diff/2001/3001#newcode2520 > include/v8.h:2520: static void > SetExecutableAllocCallback(ExecutableAllocCallback); > With the more general callback above this could take filter arguments of > type AllocationSpace, MemoryType and AllocationAction (the enums would > have to be bits then). Also consider whether more than one callback > should be allowed to be active at any one time. For GC we deprecated > SetGlobalGCPrologueCallback and SetGlobalGCEpilogueCallback in favour of > AddGCPrologueCallback/RemoveGCPrologueCallback and > AddGCEpilogueCallback/RemoveGCEpilogueCallback at some point. > > http://codereview.chromium.org/3329006/diff/2001/3003 > File src/spaces.cc (right): > > http://codereview.chromium.org/3329006/diff/2001/3003#newcode368 > src/spaces.cc:368: if (executable_alloc_callback_function_) > Please add != NULL to this type of condition. > > http://codereview.chromium.org/3329006/diff/2001/3003#newcode397 > src/spaces.cc:397: void > MemoryAllocator::SetExecutableAllocCallback(ExecutableAllocCallback f) { > Rename f to callback. > > http://codereview.chromium.org/3329006/show > > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
