Object.observe: use JSWeakMaps instead of raw ObjectHashTables in observation state

Object.observe: use JSWeakMaps instead of raw ObjectHashTables in observation state

object-observe.js uses weak maps to add "hidden" properties to
objects. Previously, the hash tables it was using weren't actually
weak. This patch changes the existing runtime functions to create
instances of JSWeakMap instead of exposing ObjectHashTable directly.

Committed: https://code.google.com/p/v8/source/detail?r=13591

[adamk, 2013-01-31 00:28:48]

Note that I haven't added any tests. The existing tests didn't depend on the
weakness of the maps. Suggestions on how to test that weakness welcome.

Also, note that I've put a TODO in ObservationWeakMapCreate: right now I create
a new map for each JSWeakMap, which seems a little wasteful, but there are only
three of them. Let me know if it's worth moving that handle to the strong roots
list now.

[Michael Starzinger, 2013-01-31 11:00:47]

LGTM. I am fine with allocating a new map on each ObservationWeakMapCreate()
call for now as long as we have the TODO in there. It also makes the keyed load
sites and some of the call sites in object-observe.js polymorphic I would
assume, but that overhead is negligible for now.

As for testing, there are unit tests in test-weakmaps.cc which test weakness.
Unfortunately there is no way from within JavaScript to test the weakness (at
least not without runtime helpers).

I would also wait for Andreas' blessing on this CL.

[adamk, 2013-01-31 21:53:02]

I've slightly modified the patch to share more of the WeakMap runtime code, by
adding a new %UnwrapGlobalProxy() function to handle the only special logic that
was in ObservationWeakMap{Get,Set}. Also slightly cleaned up the
ObservationWeakMap class in object-observe.js.

[adamk, 2013-01-31 22:21:19]

I've now added a test, but unfortunately it fails. Any idea what I might be
doing wrong?

[Michael Starzinger, 2013-02-04 11:29:14]

That is strange, the test looks OK, but the keys seem to be strongly reachable
through some other path. I'll investigate that.

[Michael Starzinger, 2013-02-04 12:20:03]

I figured out what's going on with this test case. It's a tricky issue. The
problem is that CompileRun() will return the return value of the script in the
current handle scope. Since the return value of Object.observe(o,f) is "o", the
object being observed will be strongly reachable from the HandleScope of the
test, which in turn means it cannot be collected.

The solution is either to 1) wrap the CompileRun calls in a separate HandleScope
or 2) change the return value by using something like
"CompileRun("Object.observe({}, function(){}); 0;");"

The drawback of 1) is that it might mask real leaks that happen within the
object-observe implementation due to missing HandleScopes.

The drawback of 2) is that it is, well, ugly.

Both fixes make the test run just fine. Please not that you don't need two GCs
for the WeakMaps to be cleared, just one is sufficient. That is one of the cool
things about WeakMaps. Hope this helps.

[rossberg, 2013-02-04 13:18:20]

LGTM, modulo the fix to the test case that Michael described.