Author: Remi Meier <remi.me...@gmail.com> Branch: copy-over-original2 Changeset: r401:7c2e94ae8bf1 Date: 2013-07-16 13:33 +0200 http://bitbucket.org/pypy/stmgc/changeset/7c2e94ae8bf1/
Log: new approach doing the work of copying over h_original in visit() diff --git a/c4/et.c b/c4/et.c --- a/c4/et.c +++ b/c4/et.c @@ -945,6 +945,7 @@ revision_t my_lock = d->my_lock; wlog_t *item; + dprintf(("acquire_locks\n")); assert(!stm_has_got_any_lock(d)); assert(d->public_descriptor->stolen_objects.size == 0); @@ -957,6 +958,7 @@ revision_t v; retry: assert(R->h_tid & GCFLAG_PUBLIC); + assert(R->h_tid & GCFLAG_PUBLIC_TO_PRIVATE); v = ACCESS_ONCE(R->h_revision); if (IS_POINTER(v)) /* "has a more recent revision" */ { @@ -989,7 +991,7 @@ static void CancelLocks(struct tx_descriptor *d) { wlog_t *item; - + dprintf(("cancel_locks\n")); if (!g2l_any_entry(&d->public_to_private)) return; @@ -1257,7 +1259,7 @@ revision_t cur_time; struct tx_descriptor *d = thread_descriptor; assert(d->active >= 1); - + dprintf(("CommitTransaction(%p)\n", d)); spinlock_acquire(d->public_descriptor->collection_lock, 'C'); /*committing*/ if (d->public_descriptor->stolen_objects.size != 0) stm_normalize_stolen_objects(d); @@ -1341,6 +1343,7 @@ d->active = 2; d->reads_size_limit_nonatomic = 0; update_reads_size_limit(d); + dprintf(("make_inevitable(%p)\n", d)); } static revision_t acquire_inev_mutex_and_mark_global_cur_time( diff --git a/c4/gcpage.c b/c4/gcpage.c --- a/c4/gcpage.c +++ b/c4/gcpage.c @@ -223,6 +223,7 @@ id_copy->h_tid &= ~GCFLAG_PUBLIC_TO_PRIVATE; /* see fix_outdated() */ id_copy->h_tid |= GCFLAG_VISITED; + assert(id_copy->h_tid & GCFLAG_OLD); /* XXX: may not always need tracing? */ if (!(id_copy->h_tid & GCFLAG_STUB)) @@ -236,6 +237,55 @@ } } +static gcptr copy_over_original(gcptr obj) +{ + assert(!(obj->h_tid & GCFLAG_VISITED)); + assert(!(obj->h_tid & GCFLAG_STUB)); + + if (obj->h_tid & GCFLAG_PUBLIC /* XXX: required? */ + && !(obj->h_tid & GCFLAG_PREBUILT_ORIGINAL) + && obj->h_original) { + + gcptr id_copy = (gcptr)obj->h_original; + assert(!(id_copy->h_revision & 1)); /* not head-revision itself */ + if (!(id_copy->h_tid & GCFLAG_PUBLIC)) + assert(0); + /* return NULL; */ /* could be priv_from_protected with + where backup is stolen and its h-original + points to it. */ + + assert(stmgc_size(id_copy) == stmgc_size(obj)); + /* prehash may be specific hash value for prebuilts, or 0 */ + revision_t prehash = id_copy->h_original; + assert(IMPLIES(prehash, id_copy->h_tid & GCFLAG_PREBUILT_ORIGINAL)); + /* old_tid may have prebuilt_original flags that should not be lost */ + revision_t old_tid = id_copy->h_tid; + + memcpy(id_copy, obj, stmgc_size(obj)); + assert(!((id_copy->h_tid ^ old_tid) + & (GCFLAG_BACKUP_COPY //| GCFLAG_STUB, id_copy may be stub + | GCFLAG_PUBLIC | GCFLAG_HAS_ID + | GCFLAG_PRIVATE_FROM_PROTECTED))); + id_copy->h_original = prehash; + id_copy->h_tid = old_tid & ~GCFLAG_VISITED; /* will be visited next */ + + dprintf(("copy %p over %p\n", obj, id_copy)); + + /* for those visiting later: */ + obj->h_revision = (revision_t)id_copy; + + /* mark as not old for transactions to fix their + public_to_private. Otherwise, inevitable transactions + would think their public obj was modified (also for + other transactions, but they can abort) */ + obj->h_tid &= ~GCFLAG_OLD; + + return id_copy; + } + + return NULL; +} + static void visit(gcptr *pobj) { gcptr obj = *pobj; @@ -248,7 +298,26 @@ assert(!(obj->h_tid & GCFLAG_STUB)); if (!(obj->h_tid & GCFLAG_VISITED)) { obj->h_tid &= ~GCFLAG_PUBLIC_TO_PRIVATE; /* see fix_outdated() */ + + gcptr next = copy_over_original(obj); + if (next) { + revision_t loc = (revision_t)pobj - offsetof(struct stm_object_s, + h_revision); + if ((gcptr)loc != next) + /* we don't want to set h_revision of 'next' to + 'next' itself, it was already set by + copy_over_original to a global head revision */ + *pobj = next; + obj = next; + + assert(obj->h_revision & 1); + assert(!(obj->h_tid & GCFLAG_VISITED)); + goto restart; + } + obj->h_tid |= GCFLAG_VISITED; + assert(obj->h_tid & GCFLAG_OLD); + gcptrlist_insert(&objects_to_trace, obj); keep_original_alive(obj); @@ -272,6 +341,8 @@ obj = (gcptr)(obj->h_revision - 2); if (!(obj->h_tid & GCFLAG_PUBLIC)) { prev_obj->h_tid |= GCFLAG_VISITED; + assert(prev_obj->h_tid & GCFLAG_OLD); + keep_original_alive(prev_obj); assert(*pobj == prev_obj); @@ -283,14 +354,14 @@ } } - if (!(obj->h_revision & 3)) { - /* obj is neither a stub nor a most recent revision: - completely ignore obj->h_revision */ + /* if (!(obj->h_revision & 3)) { */ + /* /\* obj is neither a stub nor a most recent revision: */ + /* completely ignore obj->h_revision *\/ */ - obj = (gcptr)obj->h_revision; - assert(obj->h_tid & GCFLAG_PUBLIC); - prev_obj->h_revision = (revision_t)obj; - } + /* obj = (gcptr)obj->h_revision; */ + /* assert(obj->h_tid & GCFLAG_PUBLIC); */ + /* prev_obj->h_revision = (revision_t)obj; */ + /* } */ *pobj = obj; goto restart; } @@ -314,10 +385,20 @@ } obj->h_tid |= GCFLAG_VISITED; - B->h_tid |= GCFLAG_VISITED; + assert(obj->h_tid & GCFLAG_OLD); assert(!(obj->h_tid & GCFLAG_STUB)); - assert(!(B->h_tid & GCFLAG_STUB)); - gcptrlist_insert2(&objects_to_trace, obj, B); + + if (B->h_tid & GCFLAG_OLD) { + B->h_tid |= GCFLAG_VISITED; + assert(!(B->h_tid & GCFLAG_STUB)); + gcptrlist_insert2(&objects_to_trace, obj, B); + } + else { + /* B was copied over its h_original */ + pobj = (gcptr *)&obj->h_revision; + obj = *pobj; + goto restart; + } if (IS_POINTER(B->h_revision)) { assert(B->h_tid & GCFLAG_PUBLIC); @@ -337,6 +418,7 @@ if (!(obj->h_tid & GCFLAG_VISITED)) { obj->h_tid &= ~GCFLAG_PUBLIC_TO_PRIVATE; /* see fix_outdated() */ obj->h_tid |= GCFLAG_VISITED; + assert(obj->h_tid & GCFLAG_OLD); gcptrlist_insert(&objects_to_trace, obj); if (IS_POINTER(obj->h_revision)) { @@ -369,9 +451,11 @@ gcptr obj; for (; pobj != pend; pobj++) { obj = *pobj; + obj->h_tid &= ~GCFLAG_VISITED; assert(obj->h_tid & GCFLAG_PREBUILT_ORIGINAL); - assert(IS_POINTER(obj->h_revision)); - visit((gcptr *)&obj->h_revision); + /* assert(IS_POINTER(obj->h_revision)); */ + + visit_keep(obj); } } @@ -397,37 +481,72 @@ static void mark_all_stack_roots(void) { + int i; + gcptr *items; struct tx_descriptor *d; + struct G2L new_public_to_private; + memset(&new_public_to_private, 0, sizeof(struct G2L)); + for (d = stm_tx_head; d; d = d->tx_next) { assert(!stm_has_got_any_lock(d)); /* the roots pushed on the shadowstack */ mark_roots(d->shadowstack, *d->shadowstack_end_ref); + /* some roots (^^^) can also be in this list, and + we may have a stolen priv_from_prot in here that, + when visited, resolves to its backup (or further) */ + items = d->old_objects_to_trace.items; + for (i = d->old_objects_to_trace.size - 1; i >= 0; i--) { + visit(&items[i]); + gcptrlist_insert(&objects_to_trace, items[i]); + } + /* the thread-local object */ visit(d->thread_local_obj_ref); visit(&d->old_thread_local_obj); /* the current transaction's private copies of public objects */ wlog_t *item; + + /* transactions need to have their pub_to_priv fixed. Otherwise, + they'll think their objects got outdated. Only absolutely + necessary for inevitable transactions (XXX check performance?). */ + dprintf(("start fixup (%p):\n", d)); + G2L_LOOP_FORWARD(d->public_to_private, item) { + gcptr R = item->addr; + gcptr L = item->val; + if (!(R->h_tid & GCFLAG_OLD)) { + /* R was copied over its original */ + gcptr new_R = (gcptr)R->h_original; + /* gcptrlist_insert(&objects_to_trace, new_R); */ + + g2l_insert(&new_public_to_private, new_R, L); + G2L_LOOP_DELETE(item); + + if (L && L->h_revision == (revision_t)R) { + L->h_revision = (revision_t)new_R; + dprintf((" fixup %p to %p <-> %p\n", R, new_R, L)); + } + else { + dprintf((" fixup %p to %p -> %p\n", R, new_R, L)); + } + } + } G2L_LOOP_END; + + /* reinsert to real pub_to_priv */ + G2L_LOOP_FORWARD(new_public_to_private, item) { + g2l_insert(&d->public_to_private, item->addr, item->val); + } G2L_LOOP_END; + g2l_clear(&new_public_to_private); + + /* now visit them */ G2L_LOOP_FORWARD(d->public_to_private, item) { /* note that 'item->addr' is also in the read set, so if it was outdated, it will be found at that time */ gcptr R = item->addr; gcptr L = item->val; - /* Objects that were not visited yet must have the PUB_TO_PRIV - flag. Except if that transaction will abort anyway, then it - may be removed from a previous major collection that didn't - fix the PUB_TO_PRIV because the transaction was going to - abort anyway: - 1. minor_collect before major collect (R->L, R is outdated, abort) - 2. major collect removes flag - 3. major collect again, same thread, no time to abort - 4. flag still removed - */ - assert(IMPLIES(!(R->h_tid & GCFLAG_VISITED) && d->active > 0, - R->h_tid & GCFLAG_PUBLIC_TO_PRIVATE)); visit_keep(R); if (L != NULL) { /* minor collection found R->L in public_to_young @@ -462,6 +581,9 @@ assert(gcptrlist_size(&d->private_from_protected) == d->num_private_from_protected_known_old); } + + if (new_public_to_private.raw_start) + g2l_delete_not_used_any_more(&new_public_to_private); } static void cleanup_for_thread(struct tx_descriptor *d) @@ -477,6 +599,8 @@ for (i = d->private_from_protected.size - 1; i >= 0; i--) { gcptr obj = items[i]; assert(obj->h_tid & GCFLAG_PRIVATE_FROM_PROTECTED); + /* we don't copy private / protected objects over prebuilts (yet) */ + assert(obj->h_tid & GCFLAG_OLD); if (!(obj->h_tid & GCFLAG_VISITED)) { /* forget 'obj' */ @@ -484,6 +608,18 @@ } } + + /* we visit old_objects_to_trace during marking and thus, they + should be up-to-date */ +#ifdef _GC_DEBUG + items = d->old_objects_to_trace.items; + for (i = d->old_objects_to_trace.size - 1; i >= 0; i--) { + gcptr obj = items[i]; + assert(obj->h_tid & GCFLAG_OLD); + assert(obj->h_tid & GCFLAG_VISITED); + } +#endif + /* If we're aborting this transaction anyway, we don't need to do * more here. */ @@ -500,15 +636,24 @@ gcptr obj = items[i]; assert(!(obj->h_tid & GCFLAG_STUB)); - /* Warning: in case the object listed is outdated and has been - replaced with a more recent revision, then it might be the - case that obj->h_revision doesn't have GCFLAG_VISITED, but - just removing it is very wrong --- we want 'd' to abort. - */ - if (obj->h_tid & GCFLAG_PRIVATE_FROM_PROTECTED) { + if (!(obj->h_tid & GCFLAG_OLD)) { + obj = (gcptr)obj->h_revision; + items[i] = obj; + } + else if (obj->h_tid & GCFLAG_PRIVATE_FROM_PROTECTED) { + /* Warning: in case the object listed is outdated and has been + replaced with a more recent revision, then it might be the + case that obj->h_revision doesn't have GCFLAG_VISITED, but + just removing it is very wrong --- we want 'd' to abort. + */ /* follow obj to its backup */ assert(IS_POINTER(obj->h_revision)); obj = (gcptr)obj->h_revision; + + /* backup copies will never be candidates for copy over + prebuilts, because there is always the priv-from-prot + object inbetween */ + assert(obj->h_tid & GCFLAG_OLD); } revision_t v = obj->h_revision; @@ -551,7 +696,7 @@ G2L_LOOP_FORWARD(d->public_to_private, item) { assert(item->addr->h_tid & GCFLAG_VISITED); assert(item->val->h_tid & GCFLAG_VISITED); - + assert(item->addr->h_tid & GCFLAG_OLD); assert(item->addr->h_tid & GCFLAG_PUBLIC); /* assert(is_private(item->val)); but in the other thread, which becomes: */ diff --git a/c4/nursery.c b/c4/nursery.c --- a/c4/nursery.c +++ b/c4/nursery.c @@ -396,6 +396,10 @@ { long i, limit = d->num_read_objects_known_old; gcptr *items = d->list_of_read_objects.items; + + if (d->active < 0) + return; // aborts anyway + assert(d->list_of_read_objects.size >= limit); if (d->active == 2) { @@ -541,8 +545,9 @@ !g2l_any_entry(&d->young_objects_outside_nursery)*/ ) { /* there is no young object */ assert(gcptrlist_size(&d->public_with_young_copy) == 0); - assert(gcptrlist_size(&d->list_of_read_objects) >= - d->num_read_objects_known_old); + assert(IMPLIES(d->active > 0, + gcptrlist_size(&d->list_of_read_objects) >= + d->num_read_objects_known_old)); assert(gcptrlist_size(&d->private_from_protected) >= d->num_private_from_protected_known_old); d->num_read_objects_known_old = diff --git a/c4/test/support.py b/c4/test/support.py --- a/c4/test/support.py +++ b/c4/test/support.py @@ -583,7 +583,9 @@ lib.stm_add_prebuilt_root(p1) def delegate_original(p1, p2): - assert p1.h_original == 0 + # no h_original or it is a prebuilt with a specified hash in h_original + assert (p1.h_original == 0) or (p1.h_tid & GCFLAG_PREBUILT_ORIGINAL) + assert p1.h_tid & GCFLAG_OLD assert p2.h_original == 0 assert p1 != p2 p2.h_original = ffi.cast("revision_t", p1) diff --git a/c4/test/test_gcpage.py b/c4/test/test_gcpage.py --- a/c4/test/test_gcpage.py +++ b/c4/test/test_gcpage.py @@ -205,13 +205,13 @@ p2 = oalloc(HDR); make_public(p2) delegate(p1, p2) delegate_original(p1, p2) - p2.h_original = ffi.cast("revision_t", p1) lib.stm_push_root(p1) major_collect() major_collect() p1b = lib.stm_pop_root() check_not_free(p1) # id copy - check_not_free(p2) + check_free_old(p2) + assert p1b == p1 def test_new_version_kill_intermediate(): @@ -273,7 +273,6 @@ delegate(p3, p4) delegate(p4, p5) rawsetptr(p1, 0, p3) - delegate_original(p3, p1) delegate_original(p3, p2) delegate_original(p3, p4) delegate_original(p3, p5) @@ -285,11 +284,8 @@ check_free_old(p2) check_not_free(p3) # original check_free_old(p4) - check_not_free(p5) - assert rawgetptr(p1, 0) == p5 - assert follow_original(p1) == p3 - assert follow_original(p5) == p3 - + check_free_old(p5) + assert rawgetptr(p1, 0) == p3 def test_prebuilt_version_1(): p1 = lib.pseudoprebuilt(HDR, 42 + HDR) @@ -308,6 +304,23 @@ check_free_old(p2) check_not_free(p3) # XXX replace with p1 +def test_prebuilt_version_2_copy_over_prebuilt(): + p1 = lib.pseudoprebuilt_with_hash(HDR, 42 + HDR, 99) + p2 = oalloc(HDR); make_public(p2) + p3 = oalloc(HDR); make_public(p3) + delegate(p1, p2) + delegate_original(p1, p2) + delegate(p2, p3) + delegate_original(p1, p3) + # added by delegate, remove, otherwise + # major_collect will not copy over prebuilt p1: + p1.h_tid &= ~GCFLAG_PUBLIC_TO_PRIVATE + major_collect() + check_prebuilt(p1) + assert lib.stm_hash(p1) == 99 + check_free_old(p2) + check_free_old(p3) + def test_prebuilt_version_to_protected(): p1 = lib.pseudoprebuilt(HDR, 42 + HDR) p2 = lib.stm_write_barrier(p1) @@ -321,6 +334,24 @@ check_prebuilt(p1) check_not_free(p2) # XXX replace with p1 +def test_prebuilt_version_to_protected_copy_over_prebuilt(): + py.test.skip("""current copy-over-prebuilt-original approach + does not work with public_prebuilt->stub->protected""") + p1 = lib.pseudoprebuilt(HDR, 42 + HDR) + p2 = lib.stm_write_barrier(p1) + lib.stm_commit_transaction() + lib.stm_begin_inevitable_transaction() + minor_collect() + p2 = lib.stm_read_barrier(p1) + assert p2 != p1 + minor_collect() + major_collect() + major_collect() + print classify(p2) + check_prebuilt(p1) + check_free_old(p2) + + def test_private(): p1 = nalloc(HDR) lib.stm_push_root(p1) @@ -396,8 +427,6 @@ print p2 major_collect() r.leave_in_parallel() - check_not_free(p2) - assert classify(p2) == "public" r.enter_in_parallel() perform_transaction(cb) r.leave_in_parallel() _______________________________________________ pypy-commit mailing list pypy-commit@python.org http://mail.python.org/mailman/listinfo/pypy-commit