Make objects embedded in optimized code weak.

Make objects embedded in optimized code weak.

This introduces a global weak hash table that maps objects embedded in
optimized code to dependent code lists. Using this table we can deoptimize
optimized code whenever a weak object embedded in the code dies.

BUG=v8:2073
R=hpayer@chromium.org, mstarzinger@chromium.org

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

[ulan, 2013-09-12 11:34:00]

Please take a look.

[Hannes Payer (out of office), 2013-10-01 13:13:50]

Already looking good...

https://codereview.chromium.org/23477061/diff/25002/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/23477061/diff/25002/src/heap.h#newcode1889
src/heap.h:1889: 
What about?
- Making weak_object_to_code, weak_object_to_code_address, and
set_weak_object_to_code private?
- Adding an InitializeWeakObjectToCodeTable which calls
set_weak_object_to_code(isolate_->heap()->undefined_value()).
- Creating the WeakHashtable when accessing the non-initialized hash table.

https://codereview.chromium.org/23477061/diff/25002/src/heap.h#newcode2388
src/heap.h:2388: Object* weak_object_to_code_;
Can we move that up to the other weak list heads, after allocation_sites_list_?
Moreover, the comment about the initialization applies also to the other weak
lists, we could add it on top of the list heads. Aaand, can we change the name
to weak_object_to_code_table_.

https://codereview.chromium.org/23477061/diff/25002/src/incremental-marking.cc
File src/incremental-marking.cc (right):

https://codereview.chromium.org/23477061/diff/25002/src/incremental-marking.c...
src/incremental-marking.cc:651:
IncrementalMarkingMarkingVisitor::MarkObjectWithoutPush(
Why don't you call MarkWeakObjectToCode here?

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc#newco...
src/mark-compact.cc:433: if (heap()->weak_embedded_maps_verification_enabled())
{
This property should be renamed to weak_embedded_objects_verification_enabled

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc#newco...
src/mark-compact.cc:2158: void
MarkCompactCollector::MarkWeakObjectToCode(RootMarkingVisitor* visitor) {
Visitor is not needed for this function.

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.h
File src/mark-compact.h (right):

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.h#newcod...
src/mark-compact.h:845: // Mark the global table from weak objects to dependent
code without
Mark the global table which maps weak objects to dependent...

https://codereview.chromium.org/23477061/diff/25002/src/x64/lithium-codegen-x...
File src/x64/lithium-codegen-x64.cc (left):

https://codereview.chromium.org/23477061/diff/25002/src/x64/lithium-codegen-x...
src/x64/lithium-codegen-x64.cc:92: if
(FLAG_weak_embedded_maps_in_optimized_code) {
Why did you remove that flag?

[ulan, 2013-10-01 14:50:29]

Thank you, addressed comments.

https://codereview.chromium.org/23477061/diff/25002/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/23477061/diff/25002/src/heap.h#newcode1889
src/heap.h:1889: 
On 2013/10/01 13:13:50, Hannes Payer wrote:
> What about?
> - Making weak_object_to_code, weak_object_to_code_address, and
> set_weak_object_to_code private?
> - Adding an InitializeWeakObjectToCodeTable which calls
> set_weak_object_to_code(isolate_->heap()->undefined_value()).
> - Creating the WeakHashtable when accessing the non-initialized hash table.
I moved these methods up to the methods of the existing weak lists (like
allocation_sites_list). We need these methods public because mark_compact
accesses the table.

https://codereview.chromium.org/23477061/diff/25002/src/heap.h#newcode2388
src/heap.h:2388: Object* weak_object_to_code_;
On 2013/10/01 13:13:50, Hannes Payer wrote:
> Can we move that up to the other weak list heads, after
allocation_sites_list_?
> Moreover, the comment about the initialization applies also to the other weak
> lists, we could add it on top of the list heads. Aaand, can we change the name
> to weak_object_to_code_table_.

Done.

https://codereview.chromium.org/23477061/diff/25002/src/incremental-marking.cc
File src/incremental-marking.cc (right):

https://codereview.chromium.org/23477061/diff/25002/src/incremental-marking.c...
src/incremental-marking.cc:651:
IncrementalMarkingMarkingVisitor::MarkObjectWithoutPush(
On 2013/10/01 13:13:50, Hannes Payer wrote:
> Why don't you call MarkWeakObjectToCode here?

Done.

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc#newco...
src/mark-compact.cc:433: if (heap()->weak_embedded_maps_verification_enabled())
{
On 2013/10/01 13:13:50, Hannes Payer wrote:
> This property should be renamed to weak_embedded_objects_verification_enabled
Can we instead rename the method to
VerifyWeakEmbeddedMapsAndObjectsInOptimizedCode()?
Renaming of the property triggers renaming of
NoWeakEmbeddedMapsVerificationScope, which makes many lines longer than 80. :)

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc#newco...
src/mark-compact.cc:2158: void
MarkCompactCollector::MarkWeakObjectToCode(RootMarkingVisitor* visitor) {
On 2013/10/01 13:13:50, Hannes Payer wrote:
> Visitor is not needed for this function.

Done.

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.h
File src/mark-compact.h (right):

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.h#newcod...
src/mark-compact.h:845: // Mark the global table from weak objects to dependent
code without
On 2013/10/01 13:13:50, Hannes Payer wrote:
> Mark the global table which maps weak objects to dependent...

Done.

https://codereview.chromium.org/23477061/diff/25002/src/x64/lithium-codegen-x...
File src/x64/lithium-codegen-x64.cc (left):

https://codereview.chromium.org/23477061/diff/25002/src/x64/lithium-codegen-x...
src/x64/lithium-codegen-x64.cc:92: if
(FLAG_weak_embedded_maps_in_optimized_code) {
On 2013/10/01 13:13:50, Hannes Payer wrote:
> Why did you remove that flag?
This check is moved inside the function in IsWeakEmbeddedObject() because we
handle both weak maps and weak objects in this 
RegisterDependentCodeForEmbeddedMaps now.

[Hannes Payer (out of office), 2013-10-02 12:21:47]

LGTM, a few renaming/reformatting nits

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/23477061/diff/25002/src/mark-compact.cc#newco...
src/mark-compact.cc:433: if (heap()->weak_embedded_maps_verification_enabled())
{
On 2013/10/01 14:50:29, ulan wrote:
> On 2013/10/01 13:13:50, Hannes Payer wrote:
> > This property should be renamed to
weak_embedded_objects_verification_enabled
> Can we instead rename the method to
> VerifyWeakEmbeddedMapsAndObjectsInOptimizedCode()?
> Renaming of the property triggers renaming of
> NoWeakEmbeddedMapsVerificationScope, which makes many lines longer than 80. :)

I think object is general enough to also reflect maps.
What about renaming the method to VerifyWeakEmbeddedObjectsInOptimizedCode()?
The scope and the field should not be called *Map* anymore, we need a shorter
name for that. Anything that is generic enough is fine with me.

https://codereview.chromium.org/23477061/diff/41001/src/handles.h
File src/handles.h (right):

https://codereview.chromium.org/23477061/diff/41001/src/handles.h#newcode324
src/handles.h:324: 
remove one newline

https://codereview.chromium.org/23477061/diff/41001/src/handles.h#newcode328
src/handles.h:328: 
remove one newline

https://codereview.chromium.org/23477061/diff/41001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/23477061/diff/41001/src/heap.h#newcode1313
src/heap.h:1313: 
remove one newline

[ulan, 2013-10-02 12:36:48]

Renamed to VerifyWeakEmbeddedObjectsInOptimizedCode in new patchset.

https://chromiumcodereview.appspot.com/23477061/diff/25002/src/mark-compact.cc
File src/mark-compact.cc (right):

https://chromiumcodereview.appspot.com/23477061/diff/25002/src/mark-compact.c...
src/mark-compact.cc:433: if (heap()->weak_embedded_maps_verification_enabled())
{
On 2013/10/02 12:21:47, Hannes Payer wrote:
> On 2013/10/01 14:50:29, ulan wrote:
> > On 2013/10/01 13:13:50, Hannes Payer wrote:
> > > This property should be renamed to
> weak_embedded_objects_verification_enabled
> > Can we instead rename the method to
> > VerifyWeakEmbeddedMapsAndObjectsInOptimizedCode()?
> > Renaming of the property triggers renaming of
> > NoWeakEmbeddedMapsVerificationScope, which makes many lines longer than 80.
:)
> 
> I think object is general enough to also reflect maps.
> What about renaming the method to VerifyWeakEmbeddedObjectsInOptimizedCode()?
> The scope and the field should not be called *Map* anymore, we need a shorter
> name for that. Anything that is generic enough is fine with me.

Done.

https://chromiumcodereview.appspot.com/23477061/diff/41001/src/handles.h
File src/handles.h (right):

https://chromiumcodereview.appspot.com/23477061/diff/41001/src/handles.h#newc...
src/handles.h:324: 
On 2013/10/02 12:21:47, Hannes Payer wrote:
> remove one newline

Done.

https://chromiumcodereview.appspot.com/23477061/diff/41001/src/handles.h#newc...
src/handles.h:328: 
On 2013/10/02 12:21:47, Hannes Payer wrote:
> remove one newline

Done.

https://chromiumcodereview.appspot.com/23477061/diff/41001/src/heap.h
File src/heap.h (right):

https://chromiumcodereview.appspot.com/23477061/diff/41001/src/heap.h#newcode...
src/heap.h:1313: 
On 2013/10/02 12:21:47, Hannes Payer wrote:
> remove one newline

Done.

[Hannes Payer (out of office), 2013-10-02 12:42:30]

And a test would be awesome.

[ulan, 2013-10-04 07:25:37]

Committed patchset #7 manually as r17102 (presubmit successful).