HashMap was replaced by List<>.
test was added.


http://codereview.chromium.org/1094002/diff/6001/7003
File include/v8.h (right):

http://codereview.chromium.org/1094002/diff/6001/7003#newcode2320
include/v8.h:2320: static int SetGCPrologueCallback(GCPrologueCallback
callback);
On 2010/03/22 14:20:28, Yury Semikhatsky wrote:
I'd rather call it AddGCPrologueCallback

Done.

http://codereview.chromium.org/1094002/diff/6001/7003#newcode2320
include/v8.h:2320: static int SetGCPrologueCallback(GCPrologueCallback
callback);
On 2010/03/22 14:48:02, Søren Gjesse wrote:
How about changing enum GCType into a bitfield
enum GCType {
   kGCTypeScavenge = 1 << 0,
   kGCTypeMarkSweep = 1 << 2,
   kGCTypeMarkCompact = 1 << 3,
   kGCTypeAll = kGCTypeScavenge | kGCTypeMarkSweep | kGCTypeMarkCompact
};

and add an argument to these AddGCXXX to indicate which GC's to
receive a
callback for

   AddGCPrologueCallback(GCType types, GCPrologueCallback callback)

This way we could use this new API for what the
SetGlobalGCPrologueCallback and
SetGlobalGCEpilogueCallback does now and be able to retire these API's
at some
point.

Done.

http://codereview.chromium.org/1094002/diff/6001/7002
File src/heap.h (right):

http://codereview.chromium.org/1094002/diff/6001/7002#newcode692
src/heap.h:692: static void SetGlobalGCPrologueCallback(GCCallback
callback) {
On 2010/03/22 14:48:02, Søren Gjesse wrote:
I don't understand this ASSERT. Please always use != NULL or == NULL
when
dealing with pointers.

Done.

http://codereview.chromium.org/1094002/diff/6001/7002#newcode696
src/heap.h:696: static void SetGlobalGCEpilogueCallback(GCCallback
callback) {
On 2010/03/22 14:48:02, Søren Gjesse wrote:
Ditto.

Done.

http://codereview.chromium.org/1094002/diff/6001/7002#newcode1062
src/heap.h:1062: template <typename T>
On 2010/03/22 14:22:33, Michail Naganov wrote:
I'd say, there is no real need here for templatizing code -- it will
just create
unnecessary code duplication. I think, storing void* is enough.

The implementation was replaced to List<>.

http://codereview.chromium.org/1094002/diff/6001/7002#newcode1073
src/heap.h:1073: void InvokeGCCallbacks(GCType gc_type, GCCallbackFlags
callback_flag) {
On 2010/03/22 14:22:33, Michail Naganov wrote:
Please put implementation in a separate include file (heap-inl.h)

Done.

http://codereview.chromium.org/1094002/diff/6001/7002#newcode1074
src/heap.h:1074: if (!isEmpty())
On 2010/03/22 14:48:02, Søren Gjesse wrote:
Please add {}' for this if.

Done.

http://codereview.chromium.org/1094002/diff/6001/7002#newcode1106
src/heap.h:1106: HashMap* gc_callbacks_;
On 2010/03/22 14:20:28, Yury Semikhatsky wrote:
why not use a linked list or a vector for that?

Done.

http://codereview.chromium.org/1094002

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply 
to this email with the words "REMOVE ME" as the subject.

Reply via email to