[pypy-commit] stmgc copy-over-original2: new approach doing the work of copying over h_original in visit()
Raemi
noreply at buildbot.pypy.org
Tue Jul 16 13:33:48 CEST 2013
Author: Remi Meier <remi.meier at 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()
More information about the pypy-commit
mailing list