Author: Remi Meier <[email protected]>
Branch: use-gcc
Changeset: r1950:58f9e6d56296
Date: 2015-09-01 16:58 +0200
http://bitbucket.org/pypy/stmgc/changeset/58f9e6d56296/
Log: test and fix for rare bug when triggering minor GC in
hashtable_lookup
It was possible that allocating the entry would trigger a minor GC.
Then, there is an old hashtable with a young entry that may not get
traced in the next collection. I chose to make fall back to a
preexisting entry in that case, but another way would be to make
stm_hashtable_write_entry ensure that there is an stm_write() on the
hobj if 'entry' is young.
diff --git a/c8/stm/core.h b/c8/stm/core.h
--- a/c8/stm/core.h
+++ b/c8/stm/core.h
@@ -267,14 +267,6 @@
return stm_object_pages + segment_num * (NB_PAGES * 4096UL);
}
-static inline long get_num_segment_containing_address(char *addr)
-{
- uintptr_t delta = addr - stm_object_pages;
- uintptr_t result = delta / (NB_PAGES * 4096UL);
- assert(result < NB_SEGMENTS);
- return result;
-}
-
static inline
struct stm_segment_info_s *get_segment(long segment_num) {
return (struct stm_segment_info_s *)REAL_ADDRESS(
diff --git a/c8/stm/hashtable.c b/c8/stm/hashtable.c
--- a/c8/stm/hashtable.c
+++ b/c8/stm/hashtable.c
@@ -285,7 +285,8 @@
if (rc > 6) {
/* we can only enter here once! If we allocate stuff, we may
run the GC, and so 'hashtableobj' might move afterwards. */
- if (_is_in_nursery(hashtableobj)) {
+ if (_is_in_nursery(hashtableobj)
+ && will_allocate_in_nursery(sizeof(stm_hashtable_entry_t))) {
/* this also means that the hashtable is from this
transaction and not visible to other segments yet, so
the new entry can be nursery-allocated. */
@@ -325,9 +326,6 @@
stm_allocate_preexisting(sizeof(stm_hashtable_entry_t),
(char *)&initial.header);
hashtable->additions++;
- /* make sure .object is NULL in all segments before
- "publishing" the entry in the hashtable: */
- write_fence();
}
table->items[i] = entry;
write_fence(); /* make sure 'table->items' is written here */
@@ -362,6 +360,9 @@
stm_write((object_t *)entry);
+ /* this restriction may be lifted, see test_new_entry_if_nursery_full:
*/
+ assert(IMPLY(_is_young((object_t *)entry), _is_young(hobj)));
+
uintptr_t i = list_count(STM_PSEGMENT->modified_old_objects);
if (i > 0 && list_item(STM_PSEGMENT->modified_old_objects, i - 3)
== (uintptr_t)entry) {
@@ -486,7 +487,7 @@
objects in the segment of the running transaction. Otherwise,
the base case is to read them all from segment zero.
*/
- long segnum = get_num_segment_containing_address((char *)hobj);
+ long segnum = get_segment_of_linear_address((char *)hobj);
if (!IS_OVERFLOW_OBJ(get_priv_segment(segnum), hobj))
segnum = 0;
diff --git a/c8/stm/nursery.h b/c8/stm/nursery.h
--- a/c8/stm/nursery.h
+++ b/c8/stm/nursery.h
@@ -27,6 +27,20 @@
get_priv_segment(other_segment_num)->safe_point != SP_RUNNING);
}
+static inline bool will_allocate_in_nursery(size_t size_rounded_up) {
+ OPT_ASSERT(size_rounded_up >= 16);
+ OPT_ASSERT((size_rounded_up & 7) == 0);
+
+ if (UNLIKELY(size_rounded_up >= _STM_FAST_ALLOC))
+ return false;
+
+ stm_char *p = STM_SEGMENT->nursery_current;
+ stm_char *end = p + size_rounded_up;
+ if (UNLIKELY((uintptr_t)end > STM_SEGMENT->nursery_end))
+ return false;
+ return true;
+}
+
#define must_abort() is_abort(STM_SEGMENT->nursery_end)
static object_t *find_shadow(object_t *obj);
diff --git a/c8/test/support.py b/c8/test/support.py
--- a/c8/test/support.py
+++ b/c8/test/support.py
@@ -12,6 +12,8 @@
#define _STM_FAST_ALLOC ...
#define _STM_CARD_SIZE ...
#define _STM_CARD_MARKED ...
+#define STM_GC_NURSERY ...
+#define SIZEOF_HASHTABLE_ENTRY ...
typedef struct {
...;
@@ -209,6 +211,9 @@
object_t *hashtable_read_result;
bool _check_hashtable_write(object_t *, stm_hashtable_t *, uintptr_t key,
object_t *nvalue, stm_thread_local_t *tl);
+stm_hashtable_entry_t *stm_hashtable_lookup(object_t *hashtableobj,
+ stm_hashtable_t *hashtable,
+ uintptr_t index);
long stm_hashtable_length_upper_bound(stm_hashtable_t *);
uint32_t stm_hashtable_entry_userdata;
void stm_hashtable_tracefn(struct object_s *, stm_hashtable_t *,
@@ -256,6 +261,7 @@
typedef TLPREFIX struct myobj_s myobj_t;
#define SIZEOF_MYOBJ sizeof(struct myobj_s)
+#define SIZEOF_HASHTABLE_ENTRY sizeof(struct stm_hashtable_entry_s)
int _stm_get_flags(object_t *obj) {
return obj->stm_flags;
@@ -580,6 +586,7 @@
('STM_NO_COND_WAIT', '1'),
('STM_DEBUGPRINT', '1'),
('_STM_NURSERY_ZEROED', '1'),
+ ('STM_GC_NURSERY', '128'), # KB
('GC_N_SMALL_REQUESTS', str(GC_N_SMALL_REQUESTS)), #check
],
undef_macros=['NDEBUG'],
@@ -600,7 +607,8 @@
CARD_MARKED = lib._STM_CARD_MARKED
CARD_MARKED_OLD = lib._stm_get_transaction_read_version
lib.stm_hashtable_entry_userdata = 421418
-
+NURSERY_SIZE = lib.STM_GC_NURSERY * 1024 # bytes
+SIZEOF_HASHTABLE_ENTRY = lib.SIZEOF_HASHTABLE_ENTRY
class Conflict(Exception):
pass
@@ -662,14 +670,20 @@
lib._set_type_id(o, tid)
return o
+SIZEOF_HASHTABLE_OBJ = 16 + lib.SIZEOF_MYOBJ
def stm_allocate_hashtable():
o = lib.stm_allocate(16)
+ assert is_in_nursery(o)
tid = 421419
lib._set_type_id(o, tid)
h = lib.stm_hashtable_create()
lib._set_hashtable(o, h)
return o
+def hashtable_lookup(hto, ht, idx):
+ return ffi.cast("object_t*",
+ lib.stm_hashtable_lookup(hto, ht, idx))
+
def get_hashtable(o):
assert lib._get_type_id(o) == 421419
h = lib._get_hashtable(o)
diff --git a/c8/test/test_hashtable.py b/c8/test/test_hashtable.py
--- a/c8/test/test_hashtable.py
+++ b/c8/test/test_hashtable.py
@@ -354,6 +354,50 @@
stm_major_collect() # to get rid of the hashtable object
+ def test_new_entry_if_nursery_full(self):
+ self.start_transaction()
+ tl0 = self.tls[self.current_thread]
+ # make sure we fill the nursery *exactly* so that
+ # the last entry allocation triggers a minor GC
+ # and needs to allocate preexisting outside the nursery:
+ SMALL = 24 + lib.SIZEOF_MYOBJ
+ assert (NURSERY_SIZE - SIZEOF_HASHTABLE_OBJ) % SMALL <
SIZEOF_HASHTABLE_ENTRY
+ to_alloc = (NURSERY_SIZE - SIZEOF_HASHTABLE_OBJ) // SMALL
+ for i in range(to_alloc):
+ stm_allocate(SMALL)
+ h = self.allocate_hashtable()
+ assert is_in_nursery(h)
+ self.push_root(h)
+ # would trigger minor GC when allocating 'entry' in nursery:
+ entry = hashtable_lookup(h, get_hashtable(h), 123)
+ h = self.pop_root()
+ self.push_root(h)
+ assert is_in_nursery(h) # didn't trigger minor-gc, since entry
allocated outside
+ assert not is_in_nursery(entry)
+ assert htget(h, 123) == ffi.NULL
+ htset(h, 123, h, tl0)
+
+ # stm_write(h) - the whole thing may be fixed also by ensuring
+ # the hashtable gets retraced in minor-GC if stm_hashtable_write_entry
+ # detects the 'entry' to be young (and hobj being old)
+
+ stm_minor_collect()
+ h = self.pop_root()
+ assert htget(h, 123) == h
+ entry2 = hashtable_lookup(h, get_hashtable(h), 123)
+ assert entry == entry2
+ assert not is_in_nursery(h)
+ assert not is_in_nursery(entry2)
+
+ # get rid of ht:
+ self.commit_transaction()
+ self.start_transaction()
+ stm_major_collect()
+ self.commit_transaction()
+
+
+
+
class TestRandomHashtable(BaseTestHashtable):
def setup_method(self, meth):
_______________________________________________
pypy-commit mailing list
[email protected]
https://mail.python.org/mailman/listinfo/pypy-commit
