Issue 11038004: - WeakProperty keys in other generations are black. Avoid trying to

Issue 11038004: - WeakProperty keys in other generations are black. Avoid trying to (Closed)

Created:
8 years, 2 months ago by Ivan Posva

Modified:
8 years, 2 months ago

Reviewers:
cshapiro, siva

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/runtime/

Visibility:
Public.

More Reviews

Description

- WeakProperty keys in other generations are black. Avoid trying to set the watch bit on keys in the VM isolate or on keys that are non-heap objects (aka Smis). - Add appropriate tests. - Fixes example code from comment 2 in issue 5546. Committed: https://code.google.com/p/dart/source/detail?r=13090

Patch Set 1 #

Total comments: 6

Created: 8 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+127 lines, -10 lines)			Patch
M	vm/gc_marker.cc	View	1 chunk	+3 lines, -1 line	2 comments	Download
M	vm/object_test.cc	View	1 chunk	+114 lines, -0 lines	4 comments	Download
M	vm/scavenger.cc	View	1 chunk	+10 lines, -9 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

cshapiro

lgtm with comments https://chromiumcodereview.appspot.com/11038004/diff/1/vm/gc_marker.cc File vm/gc_marker.cc (right): https://chromiumcodereview.appspot.com/11038004/diff/1/vm/gc_marker.cc#newcode371 vm/gc_marker.cc:371: if (raw_key->IsHeapObject() && Can we use ...

8 years, 2 months ago (2012-10-01 22:22:14 UTC) #2

Ivan Posva

8 years, 2 months ago (2012-10-01 22:27:39 UTC) #3

https://chromiumcodereview.appspot.com/11038004/diff/1/vm/gc_marker.cc
File vm/gc_marker.cc (right):

https://chromiumcodereview.appspot.com/11038004/diff/1/vm/gc_marker.cc#newcod...
vm/gc_marker.cc:371: if (raw_key->IsHeapObject() &&
On 2012/10/01 22:22:14, cshapiro wrote:
> Can we use IsUnreachable instead of the explicit conjunction?  The null check
> inside IsUnreachable should be unnecessary now that the null object is
> pre-marked.

Different CL.

https://chromiumcodereview.appspot.com/11038004/diff/1/vm/object_test.cc
File vm/object_test.cc (right):

https://chromiumcodereview.appspot.com/11038004/diff/1/vm/object_test.cc#newc...
vm/object_test.cc:2921: // Weak property key and value should survive due to
cross-generation
On 2012/10/01 22:22:14, cshapiro wrote:
> This comment is false, no?  The key is implicitly live because it is not a
heap
> object.

Done.

https://chromiumcodereview.appspot.com/11038004/diff/1/vm/object_test.cc#newc...
vm/object_test.cc:2939: // Weak property key and value should survive due to
cross-generation
On 2012/10/01 22:22:14, cshapiro wrote:
> Same here.

Done.

Expand Messages | Collapse Messages