Author: Remi Meier <remi.me...@inf.ethz.ch> 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 pypy-commit@python.org https://mail.python.org/mailman/listinfo/pypy-commit