3212 - bugfix in refcount management

When updating refcounts for a typed segment of memory being copied over
with another, we were only ever using the new copy's data to determine
any tags for exclusive containers. Looks like the right way to do
refcounts is to increment and decrement separately.

Hopefully this is a complete fix for the intermittent but
non-deterministic errors we've been encountering while running the edit/
app.
This commit is contained in:
Kartik K. Agaram 2016-08-17 12:14:09 -07:00
parent ed8e4c1741
commit 75ab873238
5 changed files with 102 additions and 71 deletions

View File

@ -276,6 +276,18 @@ int trace_count(string label, string line) {
return result;
}
int trace_count_prefix(string label, string prefix) {
if (!Trace_stream) return 0;
long result = 0;
for (vector<trace_line>::iterator p = Trace_stream->past_lines.begin(); p != Trace_stream->past_lines.end(); ++p) {
if (label == p->label) {
if (starts_with(trim(p->contents), trim(prefix)))
++result;
}
}
return result;
}
#define CHECK_TRACE_CONTAINS_ERROR() CHECK(trace_count("error") > 0)
#define CHECK_TRACE_DOESNT_CONTAIN_ERROR() \
if (trace_count("error") > 0) { \

View File

@ -27,30 +27,41 @@ if (Update_refcounts_in_write_memory)
:(code)
void update_any_refcounts(const reagent& canonized_x, const vector<double>& data) {
increment_any_refcounts(canonized_x, data); // increment first so we don't reclaim on x <- copy x
decrement_any_refcounts(canonized_x);
}
void increment_any_refcounts(const reagent& canonized_x, const vector<double>& data) {
if (is_mu_address(canonized_x)) {
assert(scalar(data));
assert(!canonized_x.metadata.size);
increment_refcount(data.at(0));
}
// End Increment Refcounts(canonized_x)
}
void increment_refcount(int new_address) {
assert(new_address >= 0);
if (new_address == 0) return;
int new_refcount = get_or_insert(Memory, new_address);
trace(9999, "mem") << "incrementing refcount of " << new_address << ": " << new_refcount << " -> " << new_refcount+1 << end();
put(Memory, new_address, new_refcount+1);
}
void decrement_any_refcounts(const reagent& canonized_x) {
if (is_mu_address(canonized_x)) {
assert(canonized_x.value);
assert(!canonized_x.metadata.size);
update_refcounts(canonized_x, data.at(0));
decrement_refcount(get_or_insert(Memory, canonized_x.value), canonized_x.type->right, payload_size(canonized_x));
}
// End Update Refcounts(canonized_x)
// End Decrement Refcounts(canonized_x)
}
void update_refcounts(const reagent& old, int new_address) {
assert(is_mu_address(old));
update_refcounts(get_or_insert(Memory, old.value), new_address, old.type->right, payload_size(old));
}
void update_refcounts(int old_address, int new_address, const type_tree* payload_type, int /*just in case it's an array*/payload_size) {
if (old_address == new_address) {
trace(9999, "mem") << "copying address to itself; refcount unchanged" << end();
return;
}
// decrement refcount of old address
void decrement_refcount(int old_address, const type_tree* payload_type, int payload_size) {
assert(old_address >= 0);
if (old_address) {
int old_refcount = get_or_insert(Memory, old_address);
trace(9999, "mem") << "decrementing refcount of " << old_address << ": " << old_refcount << " -> " << (old_refcount-1) << end();
trace(9999, "mem") << "decrementing refcount of " << old_address << ": " << old_refcount << " -> " << old_refcount-1 << end();
--old_refcount;
put(Memory, old_address, old_refcount);
if (old_refcount < 0) {
@ -66,13 +77,6 @@ void update_refcounts(int old_address, int new_address, const type_tree* payload
}
// End Decrement Refcount(old_address, payload_type, payload_size)
}
// increment refcount of new address
if (new_address) {
int new_refcount = get_or_insert(Memory, new_address);
assert(new_refcount >= 0); // == 0 only when new_address == old_address
trace(9999, "mem") << "incrementing refcount of " << new_address << ": " << new_refcount << " -> " << (new_refcount+1) << end();
put(Memory, new_address, new_refcount+1);
}
}
int payload_size(reagent/*copy*/ x) {
@ -90,7 +94,8 @@ def main [
+run: {1: ("address" "number")} <- new {number: "type"}
+mem: incrementing refcount of 1000: 0 -> 1
+run: {1: ("address" "number")} <- copy {1: ("address" "number")}
+mem: copying address to itself; refcount unchanged
+mem: incrementing refcount of 1000: 1 -> 2
+mem: decrementing refcount of 1000: 2 -> 1
:(scenario refcounts_call)
def main [
@ -175,6 +180,7 @@ def main [
+mem: incrementing refcount of 1000: 2 -> 3
:(after "Write Memory in Successful MAYBE_CONVERT")
// TODO: double-check data here as well
vector<double> data;
for (int i = 0; i < size_of(product); ++i)
data.push_back(get_or_insert(Memory, base_address+/*skip tag*/1+i));
@ -374,21 +380,33 @@ int payload_size(const type_tree* type) {
//: use metadata.address to update refcounts within containers, arrays and
//: exclusive containers
:(before "End Update Refcounts(canonized_x)")
if (is_mu_container(canonized_x) || is_mu_exclusive_container(canonized_x))
update_container_refcounts(canonized_x, data);
:(code)
void update_container_refcounts(const reagent& canonized_x, const vector<double>& data) {
assert(is_mu_container(canonized_x) || is_mu_exclusive_container(canonized_x));
:(before "End Increment Refcounts(canonized_x)")
if (is_mu_container(canonized_x) || is_mu_exclusive_container(canonized_x)) {
const container_metadata& metadata = get(Container_metadata, canonized_x.type);
for (map<set<tag_condition_info>, set<address_element_info> >::const_iterator p = metadata.address.begin(); p != metadata.address.end(); ++p) {
if (!all_match(data, p->first)) continue;
for (set<address_element_info>::const_iterator info = p->second.begin(); info != p->second.end(); ++info)
update_refcounts(get_or_insert(Memory, canonized_x.value + info->offset), data.at(info->offset), info->payload_type, size_of(info->payload_type)+/*refcount*/1);
increment_refcount(data.at(info->offset));
}
}
:(before "End Decrement Refcounts(canonized_x)")
if (is_mu_container(canonized_x) || is_mu_exclusive_container(canonized_x)) {
trace(9999, "mem") << "need to read old value to figure out what refcounts to decrement" << end();
// read from canonized_x but without canonizing again
// todo: inline without running canonize all over again
reagent/*copy*/ tmp = canonized_x;
tmp.properties.push_back(pair<string, string_tree*>("raw", NULL));
vector<double> data = read_memory(tmp);
const container_metadata& metadata = get(Container_metadata, canonized_x.type);
for (map<set<tag_condition_info>, set<address_element_info> >::const_iterator p = metadata.address.begin(); p != metadata.address.end(); ++p) {
if (!all_match(data, p->first)) continue;
for (set<address_element_info>::const_iterator info = p->second.begin(); info != p->second.end(); ++info)
decrement_refcount(get_or_insert(Memory, canonized_x.value + info->offset), info->payload_type, size_of(info->payload_type)+/*refcount*/1);
}
}
:(code)
bool all_match(const vector<double>& data, const set<tag_condition_info>& conditions) {
for (set<tag_condition_info>::const_iterator p = conditions.begin(); p != conditions.end(); ++p) {
if (data.at(p->offset) != p->tag)
@ -592,6 +610,36 @@ def main [
+run: {2: "foo"} <- merge {3: ("address" "array" "number")}
+mem: decrementing refcount of 1000: 2 -> 1
:(scenario refcounts_handle_exclusive_containers_with_different_tags)
container foo1 [
x:address:number
y:number
]
container foo2 [
x:number
y:address:number
]
exclusive-container bar [
a:foo1
b:foo2
]
def main [
1:address:number <- copy 12000/unsafe # pretend allocation
*1:address:number <- copy 34
2:bar <- merge 0/foo1, 1:address:number, 97
5:address:number <- copy 13000/unsafe # pretend allocation
*5:address:number <- copy 35
6:bar <- merge 1/foo2, 98, 5:address:number
2:bar <- copy 6:bar
]
+run: {2: "bar"} <- merge {0: "literal", "foo1": ()}, {1: ("address" "number")}, {97: "literal"}
+mem: incrementing refcount of 12000: 1 -> 2
+run: {6: "bar"} <- merge {1: "literal", "foo2": ()}, {98: "literal"}, {5: ("address" "number")}
+mem: incrementing refcount of 13000: 1 -> 2
+run: {2: "bar"} <- copy {6: "bar"}
+mem: incrementing refcount of 13000: 2 -> 3
+mem: decrementing refcount of 12000: 2 -> 1
:(code)
bool is_mu_container(const reagent& r) {
return is_mu_container(r.type);

View File

@ -36,27 +36,17 @@ void abandon(int address, const type_tree* payload_type, int payload_size) {
element.type = copy_array_element(payload_type);
int array_length = get_or_insert(Memory, address+/*skip refcount*/1);
assert(element.type->name != "array");
if (is_mu_address(element)) {
for (element.set_value(address+/*skip refcount*/1+/*skip length*/1); element.value < address+/*skip refcount*/1+/*skip length*/1+array_length; ++element.value)
update_refcounts(element, 0);
}
else if (is_mu_container(element) || is_mu_exclusive_container(element)) {
int element_size = size_of(element);
vector<double> zeros;
zeros.resize(element_size);
for (int i = 0; i < array_length; ++i) {
element.set_value(address + /*skip refcount*/1 + /*skip array length*/1 + i*element_size);
update_container_refcounts(element, zeros);
}
int element_size = size_of(element);
for (int i = 0; i < array_length; ++i) {
element.set_value(address + /*skip refcount*/1 + /*skip array length*/1 + i*element_size);
decrement_any_refcounts(element);
}
}
else if (is_mu_container(payload_type) || is_mu_exclusive_container(payload_type)) {
reagent tmp;
tmp.set_value(address + /*skip refcount*/1);
tmp.type = new type_tree(*payload_type);
vector<double> zeros;
zeros.resize(size_of(payload_type));
update_container_refcounts(tmp, zeros);
tmp.set_value(address + /*skip refcount*/1);
decrement_any_refcounts(tmp);
}
// clear memory
for (int curr = address; curr < address+payload_size; ++curr)

View File

@ -189,14 +189,19 @@ def main [
3:foo:point <- merge 0/x, 15, 16
6:foo:point <- merge 1/y, 23
]
+run: {1: ("foo" "number")} <- merge {0: "literal", "x": ()}, {34: "literal"}
+mem: storing 0 in location 1
+mem: storing 34 in location 2
+run: {3: ("foo" "point")} <- merge {0: "literal", "x": ()}, {15: "literal"}, {16: "literal"}
+mem: storing 0 in location 3
+mem: storing 15 in location 4
+mem: storing 16 in location 5
+run: {6: ("foo" "point")} <- merge {1: "literal", "y": ()}, {23: "literal"}
+mem: storing 1 in location 6
+mem: storing 23 in location 7
$mem: 7
+run: reply
# no other stores
% CHECK(trace_count_prefix("mem", "storing") == 7);
:(scenario get_on_shape_shifting_container)
container foo:_t [

View File

@ -262,30 +262,6 @@ increment_any_refcounts(ingredient, ingredients.at(i));
:(before "End should_update_refcounts_in_write_memory Special-cases For Primitives")
if (inst.operation == NEXT_INGREDIENT || inst.operation == NEXT_INGREDIENT_WITHOUT_TYPECHECKING)
return false;
:(code)
void increment_any_refcounts(const reagent& x, const vector<double>& data) {
if (is_mu_address(x)) {
assert(scalar(data));
assert(x.value);
assert(!x.metadata.size);
increment_refcount(data.at(0));
}
if (is_mu_container(x) || is_mu_exclusive_container(x)) {
const container_metadata& metadata = get(Container_metadata, x.type);
for (map<set<tag_condition_info>, set<address_element_info> >::const_iterator p = metadata.address.begin(); p != metadata.address.end(); ++p) {
if (!all_match(data, p->first)) continue;
for (set<address_element_info>::const_iterator info = p->second.begin(); info != p->second.end(); ++info)
increment_refcount(data.at(info->offset));
}
}
}
void increment_refcount(int address) {
if (address == 0) return;
int refcount = get_or_insert(Memory, address);
trace(9999, "mem") << "incrementing refcount of " << address << ": " << refcount << " -> " << refcount+1 << end();
put(Memory, address, refcount+1);
}
:(scenario start_running_returns_routine_id)
def f1 [