On 2011/07/29 08:17:23, danno wrote:
You're heading in the right direction!
Thanks for the confirmation. It is good to know that I am not missing the
point
completely. :)
There are a bunch of places in the runtime that specialize behavior
depending
on
the subclass of JSObject, using the instance_type to distinguish them (as
a
random example, take a look at JSObject::GetHeaderSize). You will need to
ensure
that your new weak map subclass is properly handled in all those cases.
As a
start, just grep for JS_OBJECT_TYPE and in the source (and maybe
JS_PROXY_TYPE,
since Andreas also did something similar when he added proxy support).
I grepped through the code and found several such occurrences. But I think
I am
close to tracking them all down by now. Especially the one in
JSObject::GetHeaderSize is easy to miss, because it is only called by the
native
API. Will upload an updated CL later today.
You're also missing some GC support that is needed for handling evacuation
from
new heap and proper handing of the mark-and-compact heap. Some
object-debugging
boilerplate is also missing.
Take a look at http://codereview.chromium.org/7089002/ which also
introduced a
new subclass of Object (but not JSObject) for an example of some of the
things
you will have to add. Take a look at mark-compact.cc and heap.cc.
I think for the "strong WeakMap" it is sufficient to reuse the visitor for
normal JSObjects, because the backing hash table is referenced by the header
which all visitors will pick up. In my opinion objects-visiting.cc:118
should
take care of that. Or am I missing something here?
Yes, once the GC "learns" about the special behavior of WeakMaps, we will
need a
separate visitor id and a new entry in the visitor dispatch table. But I
wanted
to make this change in a separate CL because it is not necessary for "strong
WeakMaps".
Please also add some GC tests (gc() calls) to your. js test to actually
trigger
the GC support. You can make the GC API available to the test case by
using
--expose-gc in the header. You can find examples of the usage here:
test/mjsunit/unbox-double-arrays.js
Fixed. Inserted two explicit invocations of gc() into the test case. Thanks
for
the hint about --expose-gc, that will be really helpful!
One last not about testing: please try to create cases in your .js test
that
trigger the runtime code paths that you add... it's not always possible,
but
worth a try :-)
Yes, especially JSObject::GetHeaderSize is giving me a hard time. I think
it is
not possible to the that from within JS. Should I add an extra cctest for
that?
That will keep you busy for a while. :-)
http://codereview.chromium.org/7529007/diff/1/src/objects.h
File src/objects.h (right):
http://codereview.chromium.org/7529007/diff/1/src/objects.h#newcode335
src/objects.h:335: V(JS_WEAKMAP_TYPE)
\
nit: WEAK_MAP is more consistent
Fixed.
http://codereview.chromium.org/7529007/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
