[pypy-commit] stmgc default: Even more complications in the decoding of abort info:

arigo noreply at buildbot.pypy.org
Fri Sep 6 21:14:58 CEST 2013


Author: Armin Rigo <arigo at tunes.org>
Branch: 
Changeset: r521:24b6be3aaa67
Date: 2013-09-06 21:12 +0200
http://bitbucket.org/pypy/stmgc/changeset/24b6be3aaa67/

Log:	Even more complications in the decoding of abort info: move the
	(missing-so-far) stm_read_barrier() later, during the call to
	stm_inspect_abort_info().

diff --git a/c4/et.c b/c4/et.c
--- a/c4/et.c
+++ b/c4/et.c
@@ -24,7 +24,7 @@
     }
     cur += sprintf(cur, "tid=%ld", stm_get_tid(obj));
     cur += sprintf(cur, " : rev=%lx : orig=%lx", 
-                   obj->h_revision, obj->h_original);
+                   (long)obj->h_revision, (long)obj->h_original);
     return tmp_buf;
 }
 
@@ -950,8 +950,8 @@
         d->longest_abort_info_time = 0;   /* out of memory! */
       else
         {
-          if (stm_decode_abort_info(d, elapsed_time,
-                                     num, d->longest_abort_info) != size)
+          if (stm_decode_abort_info(d, elapsed_time, num,
+                        (struct tx_abort_info *)d->longest_abort_info) != size)
             stm_fatalerror("during stm abort: object mutated unexpectedly\n");
 
           d->longest_abort_info_time = elapsed_time;
diff --git a/c4/extra.c b/c4/extra.c
--- a/c4/extra.c
+++ b/c4/extra.c
@@ -235,13 +235,75 @@
 }
 
 size_t stm_decode_abort_info(struct tx_descriptor *d, long long elapsed_time,
-                             int abort_reason, char *output)
+                             int abort_reason, struct tx_abort_info *output)
 {
-    /* re-encodes the abort info as a single string.
+    /* Re-encodes the abort info as a single tx_abort_info structure.
+       This struct tx_abort_info is not visible to the outside, and used
+       only as an intermediate format that is fast to generate and without
+       requiring stm_read_barrier().
+     */
+    if (output != NULL) {
+        output->signature_packed = 127;
+        output->elapsed_time = elapsed_time;
+        output->abort_reason = abort_reason;
+        output->active = d->active;
+        output->atomic = d->atomic;
+        output->count_reads = d->count_reads;
+        output->reads_size_limit_nonatomic = d->reads_size_limit_nonatomic;
+    }
+
+    long num_words = 0;
+#define WRITE_WORD(word)   {                            \
+        if (output) output->words[num_words] = (word);  \
+        num_words++;                                    \
+    }
+
+    long i;
+    for (i=0; i<d->abortinfo.size; i+=2) {
+        char *object = (char*)stm_repeat_read_barrier(d->abortinfo.items[i+0]);
+        long *fieldoffsets = (long*)d->abortinfo.items[i+1];
+        long kind, offset;
+        while (*fieldoffsets != 0) {
+            kind = *fieldoffsets++;
+            WRITE_WORD(kind);
+            if (kind < 0) {
+                /* -1 is start of sublist; -2 is end of sublist */
+                continue;
+            }
+            offset = *fieldoffsets++;
+            switch(kind) {
+            case 1:    /* signed */
+            case 2:    /* unsigned */
+                WRITE_WORD(*(long *)(object + offset));
+                break;
+            case 3:    /* a string of bytes from the target object */
+                WRITE_WORD((revision_t)*(char **)(object + offset));
+                offset = *fieldoffsets++;   /* offset of len in the string */
+                WRITE_WORD(offset);
+                break;
+            default:
+                stm_fatalerror("corrupted abort log\n");
+            }
+        }
+    }
+    WRITE_WORD(0);
+#undef WRITE_WORD
+    return sizeof(struct tx_abort_info) + (num_words - 1) * sizeof(revision_t);
+}
+
+static size_t unpack_abort_info(struct tx_descriptor *d,
+                                struct tx_abort_info *ai,
+                                char *output)
+{
+    /* Lazily decodes a struct tx_abort_info into a single plain string.
        For convenience (no escaping needed, no limit on integer
-       sizes, etc.) we follow the bittorrent format. */
+       sizes, etc.) we follow the bittorrent format.  This makes the
+       format a bit more flexible for future changes.  The struct
+       tx_abort_info is still needed as an intermediate step, because
+       the string parameters may not be readable during an abort
+       (they may be stubs).
+    */
     size_t totalsize = 0;
-    long i;
     char buffer[32];
     size_t res_size;
 #define WRITE(c)   { totalsize++; if (output) *output++=(c); }
@@ -252,75 +314,74 @@
                            }
     WRITE('l');
     WRITE('l');
-    res_size = sprintf(buffer, "i%llde", (long long)elapsed_time);
+    res_size = sprintf(buffer, "i%llde", (long long)ai->elapsed_time);
     WRITE_BUF(buffer, res_size);
-    res_size = sprintf(buffer, "i%de", (int)abort_reason);
+    res_size = sprintf(buffer, "i%de", (int)ai->abort_reason);
     WRITE_BUF(buffer, res_size);
     res_size = sprintf(buffer, "i%lde", (long)d->public_descriptor_index);
     WRITE_BUF(buffer, res_size);
-    res_size = sprintf(buffer, "i%lde", (long)d->atomic);
+    res_size = sprintf(buffer, "i%lde", (long)ai->atomic);
     WRITE_BUF(buffer, res_size);
-    res_size = sprintf(buffer, "i%de", (int)d->active);
+    res_size = sprintf(buffer, "i%de", (int)ai->active);
     WRITE_BUF(buffer, res_size);
-    res_size = sprintf(buffer, "i%lue", (unsigned long)d->count_reads);
+    res_size = sprintf(buffer, "i%lue", (unsigned long)ai->count_reads);
     WRITE_BUF(buffer, res_size);
     res_size = sprintf(buffer, "i%lue",
-                       (unsigned long)d->reads_size_limit_nonatomic);
+                       (unsigned long)ai->reads_size_limit_nonatomic);
     WRITE_BUF(buffer, res_size);
     WRITE('e');
-    for (i=0; i<d->abortinfo.size; i+=2) {
-        char *object = (char*)stm_repeat_read_barrier(d->abortinfo.items[i+0]);
-        long *fieldoffsets = (long*)d->abortinfo.items[i+1];
-        long kind, offset;
-        size_t rps_size;
+
+    revision_t *src = ai->words;
+    while (*src != 0) {
+        long signed_value;
+        unsigned long unsigned_value;
         char *rps;
+        long offset, rps_size;
 
-        while (1) {
-            kind = *fieldoffsets++;
-            if (kind <= 0) {
-                if (kind == -2) {
-                    WRITE('l');    /* '[', start of sublist */
-                    continue;
-                }
-                if (kind == -1) {
-                    WRITE('e');    /* ']', end of sublist */
-                    continue;
-                }
-                break;   /* 0, terminator */
+        switch (*src++) {
+
+        case -2:
+            WRITE('l');    /* '[', start of sublist */
+            break;
+
+        case -1:
+            WRITE('e');    /* ']', end of sublist */
+            break;
+
+        case 1:    /* signed */
+            signed_value = (long)(*src++);
+            res_size = sprintf(buffer, "i%lde", signed_value);
+            WRITE_BUF(buffer, res_size);
+            break;
+
+        case 2:    /* unsigned */
+            unsigned_value = (unsigned long)(*src++);
+            res_size = sprintf(buffer, "i%lue", unsigned_value);
+            WRITE_BUF(buffer, res_size);
+            break;
+
+        case 3:    /* a string of bytes from the target object */
+            rps = (char *)(*src++);
+            offset = *src++;
+            if (rps) {
+                rps = (char *)stm_read_barrier((gcptr)rps);
+                /* xxx a bit ad-hoc: it's a string whose length is a
+                 * long at 'rps_size'; the string data follows
+                 * immediately the length */
+                rps_size = *(long *)(rps + offset);
+                assert(rps_size >= 0);
+                res_size = sprintf(buffer, "%ld:", rps_size);
+                WRITE_BUF(buffer, res_size);
+                WRITE_BUF(rps + offset + sizeof(long), rps_size);
             }
-            offset = *fieldoffsets++;
-            switch(kind) {
-            case 1:    /* signed */
-                res_size = sprintf(buffer, "i%lde",
-                                   *(long*)(object + offset));
-                WRITE_BUF(buffer, res_size);
-                break;
-            case 2:    /* unsigned */
-                res_size = sprintf(buffer, "i%lue",
-                                   *(unsigned long*)(object + offset));
-                WRITE_BUF(buffer, res_size);
-                break;
-            case 3:    /* a string of bytes from the target object */
-                rps = *(char **)(object + offset);
-                offset = *fieldoffsets++;
-                /* XXX think of a different hack: this one doesn't really
-                   work if we see stubs! */
-                if (rps && !(((gcptr)rps)->h_tid & GCFLAG_STUB)) {
-                    /* xxx a bit ad-hoc: it's a string whose length is a
-                     * long at 'offset', following immediately the offset */
-                    rps_size = *(long *)(rps + offset);
-                    assert(rps_size >= 0);
-                    res_size = sprintf(buffer, "%zu:", rps_size);
-                    WRITE_BUF(buffer, res_size);
-                    WRITE_BUF(rps + offset + sizeof(long), rps_size);
-                }
-                else {
-                    WRITE_BUF("0:", 2);
-                }
-                break;
-            default:
-                stm_fatalerror("corrupted abort log\n");
+            else {
+                /* write NULL as an empty string, good enough for now */
+                WRITE_BUF("0:", 2);
             }
+            break;
+
+        default:
+            stm_fatalerror("corrupted abort log\n");
         }
     }
     WRITE('e');
@@ -335,6 +396,53 @@
     struct tx_descriptor *d = thread_descriptor;
     if (d->longest_abort_info_time <= 0)
         return NULL;
+
+    struct tx_abort_info *ai = (struct tx_abort_info *)d->longest_abort_info;
+    assert(ai->signature_packed == 127);
+
+    stm_become_inevitable("stm_inspect_abort_info");
+
+    size_t size = unpack_abort_info(d, ai, NULL);
+    char *text = malloc(size);
+    if (text == NULL)
+        return NULL;   /* out of memory */
+    if (unpack_abort_info(d, ai, text) != size)
+        stm_fatalerror("stm_inspect_abort_info: "
+                       "object mutated unexpectedly\n");
+    free(ai);
+    d->longest_abort_info = text;
     d->longest_abort_info_time = 0;
     return d->longest_abort_info;
 }
+
+void stm_visit_abort_info(struct tx_descriptor *d, void (*visit)(gcptr *))
+{
+    long i, size = d->abortinfo.size;
+    gcptr *items = d->abortinfo.items;
+    for (i = 0; i < size; i += 2) {
+        visit(&items[i]);
+        /* items[i+1] is not a gc ptr */
+    }
+
+    struct tx_abort_info *ai = (struct tx_abort_info *)d->longest_abort_info;
+    if (ai != NULL && ai->signature_packed == 127) {
+        revision_t *src = ai->words;
+        while (*src != 0) {
+            gcptr *rpps;
+
+            switch (*src++) {
+
+            case 1:    /* signed */
+            case 2:    /* unsigned */
+                src++;        /* ignore the value */
+                break;
+
+            case 3:
+                rpps = (gcptr *)(src++);
+                src++;        /* ignore the offset */
+                visit(rpps);  /* visit() the string object */
+                break;
+            }
+        }
+    }
+}
diff --git a/c4/extra.h b/c4/extra.h
--- a/c4/extra.h
+++ b/c4/extra.h
@@ -2,8 +2,20 @@
 #define _SRCSTM_EXTRA_H
 
 
+struct tx_abort_info {
+    char signature_packed;  /* 127 when the abort_info is in this format */
+    long long elapsed_time;
+    int abort_reason;
+    int active;
+    long atomic;
+    unsigned long count_reads;
+    unsigned long reads_size_limit_nonatomic;
+    revision_t words[1];    /* the 'words' list is a bytecode-like format */
+};
+
 void stm_copy_to_old_id_copy(gcptr obj, gcptr id);
 size_t stm_decode_abort_info(struct tx_descriptor *d, long long elapsed_time,
-                             int abort_reason, char *output);
+                             int abort_reason, struct tx_abort_info *output);
+void stm_visit_abort_info(struct tx_descriptor *d, void (*visit)(gcptr *));
 
 #endif
diff --git a/c4/gcpage.c b/c4/gcpage.c
--- a/c4/gcpage.c
+++ b/c4/gcpage.c
@@ -562,12 +562,7 @@
         visit_take_protected(&d->old_thread_local_obj);
 
         /* the abortinfo objects */
-        long i, size = d->abortinfo.size;
-        gcptr *items = d->abortinfo.items;
-        for (i = 0; i < size; i += 2) {
-            visit_take_protected(&items[i]);
-            /* items[i+1] is not a gc ptr */
-        }
+        stm_visit_abort_info(d, &visit_take_protected);
 
         /* the current transaction's private copies of public objects */
         wlog_t *item;
@@ -600,8 +595,8 @@
         } G2L_LOOP_END;
 
         /* reinsert to real pub_to_priv */
-        size = new_public_to_private.size;
-        items = new_public_to_private.items;
+        long i, size = new_public_to_private.size;
+        gcptr *items = new_public_to_private.items;
         for (i = 0; i < size; i += 2) {
             g2l_insert(&d->public_to_private, items[i], items[i + 1]);
         }
diff --git a/c4/nursery.c b/c4/nursery.c
--- a/c4/nursery.c
+++ b/c4/nursery.c
@@ -455,12 +455,7 @@
     visit_if_young(d->thread_local_obj_ref);
     visit_if_young(&d->old_thread_local_obj);
 
-    long i, size = d->abortinfo.size;
-    gcptr *items = d->abortinfo.items;
-    for (i = 0; i < size; i += 2) {
-        visit_if_young(&items[i]);
-        /* items[i+1] is not a gc ptr */
-    }
+    stm_visit_abort_info(d, &visit_if_young);
 }
 
 static void minor_collect(struct tx_descriptor *d)
diff --git a/c4/stmgc.h b/c4/stmgc.h
--- a/c4/stmgc.h
+++ b/c4/stmgc.h
@@ -154,7 +154,7 @@
    stm_inspect_abort_info().  (XXX details not documented yet) */
 void stm_abort_info_push(gcptr obj, long fieldoffsets[]);
 void stm_abort_info_pop(long count);
-char *stm_inspect_abort_info(void);
+char *stm_inspect_abort_info(void);    /* turns inevitable */
 
 /* mostly for debugging support */
 void stm_abort_and_retry(void);
diff --git a/c4/test/test_extra.py b/c4/test/test_extra.py
--- a/c4/test/test_extra.py
+++ b/c4/test/test_extra.py
@@ -19,6 +19,8 @@
     # no real test here
 
 def test_inspect_abort_info_signed():
+    c = lib.stm_inspect_abort_info()
+    assert not c
     fo1 = ffi.new("long[]", [-2, 1, HDR, -1, 0])
     #
     @perform_transaction
@@ -32,6 +34,8 @@
             c = lib.stm_inspect_abort_info()
             assert c
             assert ffi.string(c).endswith("eli-421289712eee")
+            c = lib.stm_inspect_abort_info()
+            assert not c
 
 def test_inspect_abort_info_nested_unsigned():
     fo1 = ffi.new("long[]", [-2, 2, HDR, 0])


More information about the pypy-commit mailing list