[vm, gc] Fix a bug handling WeakProperties under the combination of concurrent marking and write-barrier elimination.

The compiler will in certain cases eliminate write barriers and the runtime will compensate by adding certain objects to a deferred marking stack. These objects will be revisited when marking is finalized. There is no header bit indicating an object has already been added to the deferred marking stack. The same object may be seen by markers multiple times: once as part of ordinary marking and several times as part of deferred marking.

When visiting a WeakProperty, if the fate of its key is not yet known, it is placed in a (worker-local) delayed weak properties list, which is a linked list threaded through the WeakProperties themselves. If more than one worker adds the same WeakProperty to its list, or a single worker does so multiple times, the lists will be corrupted. A previous fix (40cd5fc5f6) attempted to avoid this by ensuring only the worker which marked the WeakProperty would add it to a delayed weak properties list, but this can result in no worker adding the WeakProperty if the WeakProperty is allocated marked and added to the deferred marking stack.

This CL changes the processing of the deferred marking stack to treat WeakProperties as strong references. This ensures a WeakProperty will only be added to a delayed weak properties list by the single worker that encounters it during ordinary marking. This is also very similar to what would happen if the write barrier had not been eliminated: the writes into the marked WeakProperty would cause the new key and value to also become marked.

This CL also fixes another bug where when processing the delayed weak properties lists, the worker would try to ask whether a new-space object was marked.

TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/47128
Change-Id: Id9abaeb4dd52538ebc22ac58e48abcb7ed760854
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212612
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
This commit is contained in:
Ryan Macnak 2021-09-08 17:40:19 +00:00 committed by commit-bot@chromium.org
parent f9b7ed74dc
commit 31a1fd4b6b
5 changed files with 737 additions and 29 deletions

View file

@ -0,0 +1,352 @@
// Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// This test is a copy of the Splay benchmark that is run with a variety of
// different GC options. It makes for a good GC stress test because it
// continuously makes small changes to a large, long-lived data structure,
// stressing lots of combinations of references between new-gen and old-gen
// objects, and between marked and unmarked objects.
// The ephemeron variant of this test replaces the direct child pointers in the
// tree with Expandos to stress the handling of WeakProperties/ephemerons.
// VMOptions=
// VMOptions=--no_concurrent_mark --no_concurrent_sweep
// VMOptions=--no_concurrent_mark --concurrent_sweep
// VMOptions=--no_concurrent_mark --use_compactor
// VMOptions=--no_concurrent_mark --use_compactor --force_evacuation
// VMOptions=--concurrent_mark --no_concurrent_sweep
// VMOptions=--concurrent_mark --concurrent_sweep
// VMOptions=--concurrent_mark --use_compactor
// VMOptions=--concurrent_mark --use_compactor --force_evacuation
// VMOptions=--scavenger_tasks=0
// VMOptions=--scavenger_tasks=1
// VMOptions=--scavenger_tasks=2
// VMOptions=--scavenger_tasks=3
// VMOptions=--verify_before_gc
// VMOptions=--verify_after_gc
// VMOptions=--verify_before_gc --verify_after_gc
// VMOptions=--verify_store_buffer
// VMOptions=--stress_write_barrier_elimination
// VMOptions=--old_gen_heap_size=150
import "dart:math";
import 'package:benchmark_harness/benchmark_harness.dart';
void main() {
Splay.main();
}
class Splay extends BenchmarkBase {
const Splay() : super("Splay");
// Configuration.
static final int kTreeSize = 8000;
static final int kTreeModifications = 80;
static final int kTreePayloadDepth = 5;
static SplayTree? tree;
static Random rnd = new Random(12345);
// Insert new node with a unique key.
static num insertNewNode() {
num key;
final localTree = tree!;
do {
key = rnd.nextDouble();
} while (localTree.find(key) != null);
Payload payload = Payload.generate(kTreePayloadDepth, key.toString());
localTree.insert(key, payload);
return key;
}
static void mysetup() {
tree = new SplayTree();
for (int i = 0; i < kTreeSize; i++) insertNewNode();
}
static void tearDown() {
// Allow the garbage collector to reclaim the memory
// used by the splay tree no matter how we exit the
// tear down function.
List<num> keys = tree!.exportKeys();
tree = null;
// Verify that the splay tree has the right size.
int length = keys.length;
if (length != kTreeSize) throw new Error("Splay tree has wrong size");
// Verify that the splay tree has sorted, unique keys.
for (int i = 0; i < length - 1; i++) {
if (keys[i] >= keys[i + 1]) throw new Error("Splay tree not sorted");
}
}
void warmup() {
exercise();
}
void exercise() {
// Replace a few nodes in the splay tree.
final localTree = tree!;
for (int i = 0; i < kTreeModifications; i++) {
num key = insertNewNode();
Node? greatest = localTree.findGreatestLessThan(key);
if (greatest == null) {
localTree.remove(key);
} else {
localTree.remove(greatest.key);
}
}
}
static void main() {
mysetup();
new Splay().report();
tearDown();
}
}
class Leaf {
Leaf(String tag) :
string = "String for key $tag in leaf node",
array = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 ]
{}
String string;
List<num> array;
}
class Payload {
Payload(left, right) {
this.left = left;
this.right = right;
}
// This ordering of fields is delibrate: one key is visited before the expando
// and one after.
final leftKey = new Object();
final expando = new Expando();
final rightKey = new Object();
get left => expando[leftKey];
set left(value) => expando[leftKey] = value;
get right => expando[rightKey];
set right(value) => expando[rightKey] = value;
static generate(depth, tag) {
if (depth == 0) return new Leaf(tag);
return new Payload(generate(depth - 1, tag),
generate(depth - 1, tag));
}
}
class Error implements Exception {
const Error(this.message);
final String message;
}
/**
* A splay tree is a self-balancing binary search tree with the additional
* property that recently accessed elements are quick to access again.
* It performs basic operations such as insertion, look-up and removal
* in O(log(n)) amortized time.
*/
class SplayTree {
SplayTree();
/**
* Inserts a node into the tree with the specified [key] and value if
* the tree does not already contain a node with the specified key. If
* the value is inserted, it becomes the root of the tree.
*/
void insert(num key, value) {
if (isEmpty) {
root = new Node(key, value);
return;
}
// Splay on the key to move the last node on the search path for
// the key to the root of the tree.
splay(key);
if (root!.key == key) return;
Node node = new Node(key, value);
if (key > root!.key) {
node.left = root;
node.right = root!.right;
root!.right = null;
} else {
node.right = root;
node.left = root!.left;
root!.left = null;
}
root = node;
}
/**
* Removes a node with the specified key from the tree if the tree
* contains a node with this key. The removed node is returned. If
* [key] is not found, an exception is thrown.
*/
Node remove(num key) {
if (isEmpty) throw new Error('Key not found: $key');
splay(key);
if (root!.key != key) throw new Error('Key not found: $key');
Node removed = root!;
if (root!.left == null) {
root = root!.right;
} else {
Node? right = root!.right;
root = root!.left;
// Splay to make sure that the new root has an empty right child.
splay(key);
// Insert the original right child as the right child of the new
// root.
root!.right = right;
}
return removed;
}
/**
* Returns the node having the specified [key] or null if the tree doesn't
* contain a node with the specified [key].
*/
Node? find(num key) {
if (isEmpty) return null;
splay(key);
return root!.key == key ? root : null;
}
/**
* Returns the Node having the maximum key value.
*/
Node? findMax([Node? start]) {
if (isEmpty) return null;
Node current = null == start ? root! : start;
while (current.right != null) current = current.right!;
return current;
}
/**
* Returns the Node having the maximum key value that
* is less than the specified [key].
*/
Node? findGreatestLessThan(num key) {
if (isEmpty) return null;
// Splay on the key to move the node with the given key or the last
// node on the search path to the top of the tree.
splay(key);
// Now the result is either the root node or the greatest node in
// the left subtree.
if (root!.key < key) return root;
if (root!.left != null) return findMax(root!.left);
return null;
}
/**
* Perform the splay operation for the given key. Moves the node with
* the given key to the top of the tree. If no node has the given
* key, the last node on the search path is moved to the top of the
* tree. This is the simplified top-down splaying algorithm from:
* "Self-adjusting Binary Search Trees" by Sleator and Tarjan
*/
void splay(num key) {
if (isEmpty) return;
// Create a dummy node. The use of the dummy node is a bit
// counter-intuitive: The right child of the dummy node will hold
// the L tree of the algorithm. The left child of the dummy node
// will hold the R tree of the algorithm. Using a dummy node, left
// and right will always be nodes and we avoid special cases.
final Node dummy = new Node(0, null);
Node left = dummy;
Node right = dummy;
Node current = root!;
while (true) {
if (key < current.key) {
if (current.left == null) break;
if (key < current.left!.key) {
// Rotate right.
Node tmp = current.left!;
current.left = tmp.right;
tmp.right = current;
current = tmp;
if (current.left == null) break;
}
// Link right.
right.left = current;
right = current;
current = current.left!;
} else if (key > current.key) {
if (current.right == null) break;
if (key > current.right!.key) {
// Rotate left.
Node tmp = current.right!;
current.right = tmp.left;
tmp.left = current;
current = tmp;
if (current.right == null) break;
}
// Link left.
left.right = current;
left = current;
current = current.right!;
} else {
break;
}
}
// Assemble.
left.right = current.left;
right.left = current.right;
current.left = dummy.right;
current.right = dummy.left;
root = current;
}
/**
* Returns a list with all the keys of the tree.
*/
List<num> exportKeys() {
List<num> result = [];
if (!isEmpty) root!.traverse((Node node) => result.add(node.key));
return result;
}
// Tells whether the tree is empty.
bool get isEmpty => null == root;
// Pointer to the root node of the tree.
Node? root;
}
class Node {
Node(this.key, this.value);
final num key;
final Object? value;
// This ordering of fields is delibrate: one key is visited before the expando
// and one after.
final leftKey = new Object();
final expando = new Expando<Node>();
final rightKey = new Object();
Node? get left => expando[leftKey];
set left(Node? value) => expando[leftKey] = value;
Node? get right => expando[rightKey];
set right(Node? value) => expando[rightKey] = value;
/**
* Performs an ordered traversal of the subtree starting here.
*/
void traverse(void f(Node n)) {
Node? current = this;
while (current != null) {
Node? left = current.left;
if (left != null) left.traverse(f);
f(current);
current = current.right;
}
}
}

View file

@ -2,13 +2,11 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// This benchmark is based on a JavaScript log processing module used
// by the V8 profiler to generate execution time profiles for runs of
// JavaScript applications, and it effectively measures how fast the
// JavaScript engine is at allocating nodes and reclaiming the memory
// used for old nodes. Because of the way splay trees work, the engine
// also has to deal with a lot of changes to the large tree object
// graph.
// This test is a copy of the Splay benchmark that is run with a variety of
// different GC options. It makes for a good GC stress test because it
// continuously makes small changes to a large, long-lived data structure,
// stressing lots of combinations of references between new-gen and old-gen
// objects, and between marked and unmarked objects.
// VMOptions=
// VMOptions=--no_concurrent_mark --no_concurrent_sweep

View file

@ -0,0 +1,351 @@
// Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// This test is a copy of the Splay benchmark that is run with a variety of
// different GC options. It makes for a good GC stress test because it
// continuously makes small changes to a large, long-lived data structure,
// stressing lots of combinations of references between new-gen and old-gen
// objects, and between marked and unmarked objects.
// The ephemeron variant of this test replaces the direct child pointers in the
// tree with Expandos to stress the handling of WeakProperties/ephemerons.
// This file is copied into another directory and the default opt out scheme of
// CFE using the pattern 'vm/dart_2' doesn't work, so opt it out explicitly.
// @dart=2.9
// VMOptions=
// VMOptions=--no_concurrent_mark --no_concurrent_sweep
// VMOptions=--no_concurrent_mark --concurrent_sweep
// VMOptions=--no_concurrent_mark --use_compactor
// VMOptions=--no_concurrent_mark --use_compactor --force_evacuation
// VMOptions=--concurrent_mark --no_concurrent_sweep
// VMOptions=--concurrent_mark --concurrent_sweep
// VMOptions=--concurrent_mark --use_compactor
// VMOptions=--concurrent_mark --use_compactor --force_evacuation
// VMOptions=--scavenger_tasks=0
// VMOptions=--scavenger_tasks=1
// VMOptions=--scavenger_tasks=2
// VMOptions=--scavenger_tasks=3
// VMOptions=--verify_before_gc
// VMOptions=--verify_after_gc
// VMOptions=--verify_before_gc --verify_after_gc
// VMOptions=--verify_store_buffer
// VMOptions=--stress_write_barrier_elimination
// VMOptions=--old_gen_heap_size=150
import "dart:math";
import 'package:benchmark_harness/benchmark_harness.dart';
void main() {
Splay.main();
}
class Splay extends BenchmarkBase {
const Splay() : super("Splay");
// Configuration.
static final int kTreeSize = 8000;
static final int kTreeModifications = 80;
static final int kTreePayloadDepth = 5;
static SplayTree tree;
static Random rnd = new Random(12345);
// Insert new node with a unique key.
static num insertNewNode() {
num key;
do {
key = rnd.nextDouble();
} while (tree.find(key) != null);
Payload payload = Payload.generate(kTreePayloadDepth, key.toString());
tree.insert(key, payload);
return key;
}
static void mysetup() {
tree = new SplayTree();
for (int i = 0; i < kTreeSize; i++) insertNewNode();
}
static void tearDown() {
// Allow the garbage collector to reclaim the memory
// used by the splay tree no matter how we exit the
// tear down function.
List<num> keys = tree.exportKeys();
tree = null;
// Verify that the splay tree has the right size.
int length = keys.length;
if (length != kTreeSize) throw new Error("Splay tree has wrong size");
// Verify that the splay tree has sorted, unique keys.
for (int i = 0; i < length - 1; i++) {
if (keys[i] >= keys[i + 1]) throw new Error("Splay tree not sorted");
}
}
void warmup() {
exercise();
}
void exercise() {
// Replace a few nodes in the splay tree.
for (int i = 0; i < kTreeModifications; i++) {
num key = insertNewNode();
Node greatest = tree.findGreatestLessThan(key);
if (greatest == null) tree.remove(key);
else tree.remove(greatest.key);
}
}
static void main() {
mysetup();
new Splay().report();
tearDown();
}
}
class Leaf {
Leaf(String tag) {
string = "String for key $tag in leaf node";
array = [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 ];
}
String string;
List<num> array;
}
class Payload {
Payload(left, right) {
this.left = left;
this.right = right;
}
// This ordering of fields is delibrate: one key is visited before the expando
// and one after.
final leftKey = new Object();
final expando = new Expando();
final rightKey = new Object();
get left => expando[leftKey];
set left(value) => expando[leftKey] = value;
get right => expando[rightKey];
set right(value) => expando[rightKey] = value;
static generate(depth, tag) {
if (depth == 0) return new Leaf(tag);
return new Payload(generate(depth - 1, tag),
generate(depth - 1, tag));
}
}
class Error implements Exception {
const Error(this.message);
final String message;
}
/**
* A splay tree is a self-balancing binary search tree with the additional
* property that recently accessed elements are quick to access again.
* It performs basic operations such as insertion, look-up and removal
* in O(log(n)) amortized time.
*/
class SplayTree {
SplayTree();
/**
* Inserts a node into the tree with the specified [key] and value if
* the tree does not already contain a node with the specified key. If
* the value is inserted, it becomes the root of the tree.
*/
void insert(num key, value) {
if (isEmpty) {
root = new Node(key, value);
return;
}
// Splay on the key to move the last node on the search path for
// the key to the root of the tree.
splay(key);
if (root.key == key) return;
Node node = new Node(key, value);
if (key > root.key) {
node.left = root;
node.right = root.right;
root.right = null;
} else {
node.right = root;
node.left = root.left;
root.left = null;
}
root = node;
}
/**
* Removes a node with the specified key from the tree if the tree
* contains a node with this key. The removed node is returned. If
* [key] is not found, an exception is thrown.
*/
Node remove(num key) {
if (isEmpty) throw new Error('Key not found: $key');
splay(key);
if (root.key != key) throw new Error('Key not found: $key');
Node removed = root;
if (root.left == null) {
root = root.right;
} else {
Node right = root.right;
root = root.left;
// Splay to make sure that the new root has an empty right child.
splay(key);
// Insert the original right child as the right child of the new
// root.
root.right = right;
}
return removed;
}
/**
* Returns the node having the specified [key] or null if the tree doesn't
* contain a node with the specified [key].
*/
Node find(num key) {
if (isEmpty) return null;
splay(key);
return root.key == key ? root : null;
}
/**
* Returns the Node having the maximum key value.
*/
Node findMax([Node start]) {
if (isEmpty) return null;
Node current = null == start ? root : start;
while (current.right != null) current = current.right;
return current;
}
/**
* Returns the Node having the maximum key value that
* is less than the specified [key].
*/
Node findGreatestLessThan(num key) {
if (isEmpty) return null;
// Splay on the key to move the node with the given key or the last
// node on the search path to the top of the tree.
splay(key);
// Now the result is either the root node or the greatest node in
// the left subtree.
if (root.key < key) return root;
if (root.left != null) return findMax(root.left);
return null;
}
/**
* Perform the splay operation for the given key. Moves the node with
* the given key to the top of the tree. If no node has the given
* key, the last node on the search path is moved to the top of the
* tree. This is the simplified top-down splaying algorithm from:
* "Self-adjusting Binary Search Trees" by Sleator and Tarjan
*/
void splay(num key) {
if (isEmpty) return;
// Create a dummy node. The use of the dummy node is a bit
// counter-intuitive: The right child of the dummy node will hold
// the L tree of the algorithm. The left child of the dummy node
// will hold the R tree of the algorithm. Using a dummy node, left
// and right will always be nodes and we avoid special cases.
final Node dummy = new Node(null, null);
Node left = dummy;
Node right = dummy;
Node current = root;
while (true) {
if (key < current.key) {
if (current.left == null) break;
if (key < current.left.key) {
// Rotate right.
Node tmp = current.left;
current.left = tmp.right;
tmp.right = current;
current = tmp;
if (current.left == null) break;
}
// Link right.
right.left = current;
right = current;
current = current.left;
} else if (key > current.key) {
if (current.right == null) break;
if (key > current.right.key) {
// Rotate left.
Node tmp = current.right;
current.right = tmp.left;
tmp.left = current;
current = tmp;
if (current.right == null) break;
}
// Link left.
left.right = current;
left = current;
current = current.right;
} else {
break;
}
}
// Assemble.
left.right = current.left;
right.left = current.right;
current.left = dummy.right;
current.right = dummy.left;
root = current;
}
/**
* Returns a list with all the keys of the tree.
*/
List<num> exportKeys() {
List<num> result = [];
if (!isEmpty) root.traverse((Node node) => result.add(node.key));
return result;
}
// Tells whether the tree is empty.
bool get isEmpty => null == root;
// Pointer to the root node of the tree.
Node root;
}
class Node {
Node(this.key, this.value);
final num key;
final Object value;
// This ordering of fields is delibrate: one key is visited before the expando
// and one after.
final leftKey = new Object();
final expando = new Expando<Node>();
final rightKey = new Object();
Node get left => expando[leftKey];
set left(Node value) => expando[leftKey] = value;
Node get right => expando[rightKey];
set right(Node value) => expando[rightKey] = value;
/**
* Performs an ordered traversal of the subtree starting here.
*/
void traverse(void f(Node n)) {
Node current = this;
while (current != null) {
Node left = current.left;
if (left != null) left.traverse(f);
f(current);
current = current.right;
}
}
}

View file

@ -2,14 +2,12 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// This benchmark is based on a JavaScript log processing module used
// by the V8 profiler to generate execution time profiles for runs of
// JavaScript applications, and it effectively measures how fast the
// JavaScript engine is at allocating nodes and reclaiming the memory
// used for old nodes. Because of the way splay trees work, the engine
// also has to deal with a lot of changes to the large tree object
// graph.
//
// This test is a copy of the Splay benchmark that is run with a variety of
// different GC options. It makes for a good GC stress test because it
// continuously makes small changes to a large, long-lived data structure,
// stressing lots of combinations of references between new-gen and old-gen
// objects, and between marked and unmarked objects.
// This file is copied into another directory and the default opt out scheme of
// CFE using the pattern 'vm/dart_2' doesn't work, so opt it out explicitly.
// @dart=2.9

View file

@ -46,7 +46,7 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
void AddMicros(int64_t micros) { marked_micros_ += micros; }
bool ProcessPendingWeakProperties() {
bool marked = false;
bool more_to_mark = false;
WeakPropertyPtr cur_weak = delayed_weak_properties_;
delayed_weak_properties_ = WeakProperty::null();
while (cur_weak != WeakProperty::null()) {
@ -55,10 +55,11 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
ObjectPtr raw_key = cur_weak->untag()->key();
// Reset the next pointer in the weak property.
cur_weak->untag()->next_ = WeakProperty::null();
if (raw_key->untag()->IsMarked()) {
if (raw_key->IsSmiOrNewObject() || raw_key->untag()->IsMarked()) {
ObjectPtr raw_val = cur_weak->untag()->value();
marked = marked ||
(raw_val->IsHeapObject() && !raw_val->untag()->IsMarked());
if (!raw_val->IsSmiOrNewObject() && !raw_val->untag()->IsMarked()) {
more_to_mark = true;
}
// The key is marked so we make sure to properly visit all pointers
// originating from this weak property.
@ -70,7 +71,7 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
// Advance to next weak property in the queue.
cur_weak = next_weak;
}
return marked;
return more_to_mark;
}
void DrainMarkingStack() {
@ -175,16 +176,24 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
ObjectPtr raw_obj;
while ((raw_obj = deferred_work_list_.Pop()) != nullptr) {
ASSERT(raw_obj->IsHeapObject() && raw_obj->IsOldObject());
// N.B. We are scanning the object even if it is already marked.
// We need to scan objects even if they were already scanned via ordinary
// marking. An object may have changed since its ordinary scan and been
// added to deferred marking stack to compensate for write-barrier
// elimination.
// A given object may be included in the deferred marking stack multiple
// times. It may or may not also be in the ordinary marking stack, so
// failing to acquire the mark bit here doesn't reliably indicate the
// object was already encountered through the deferred marking stack. Our
// processing here is idempotent, so repeated visits only hurt performance
// but not correctness. Duplicatation is expected to be low.
// By the absence of a special case, we are treating WeakProperties as
// strong references here. This guarentees a WeakProperty will only be
// added to the delayed_weak_properties_ list of the worker that
// encounters it during ordinary marking. This is in the same spirit as
// the eliminated write barrier, which would have added the newly written
// key and value to the ordinary marking stack.
bool did_mark = TryAcquireMarkBit(raw_obj);
const intptr_t class_id = raw_obj->GetClassId();
intptr_t size;
if (class_id != kWeakPropertyCid) {
size = raw_obj->untag()->VisitPointersNonvirtual(this);
} else {
WeakPropertyPtr raw_weak = static_cast<WeakPropertyPtr>(raw_obj);
size = ProcessWeakProperty(raw_weak, did_mark);
}
intptr_t size = raw_obj->untag()->VisitPointersNonvirtual(this);
// Add the size only if we win the marking race to prevent
// double-counting.
if (did_mark) {