Merge branch 'reimpl-dict'

include/pocketpy/interpreter/types.h

@@ -3,26 +3,22 @@
 #include "pocketpy/common/vector.h"
 #include "pocketpy/objects/base.h"
 
-#define PK_DICT_MAX_COLLISION 4
-
 typedef struct {
     uint64_t hash;
     py_TValue key;
     py_TValue val;
 } DictEntry;
 
-typedef struct {
-    int _[PK_DICT_MAX_COLLISION];
-} DictIndex;
-
 typedef struct {
     int length;
     uint32_t capacity;
-    DictIndex* indices;
+    uint32_t null_index_value;
+    bool index_is_short;
+    void* indices;
     c11_vector /*T=DictEntry*/ entries;
 } Dict;
 
 typedef c11_vector List;
 
 void c11_chunked_array2d__mark(void* ud, c11_vector* p_stack);
-void function__gc_mark(void* ud, c11_vector* p_stack);
+void function__gc_mark(void* ud, c11_vector* p_stack);

include/pocketpy/xmacros/fixedhash.h

@@ -124,3 +124,4 @@ bool METHOD(contains)(NAME* self, K key) {
 #undef less
 #undef partial_less
 #undef equal
+#undef hash

src/objects/namedict.c

@@ -8,7 +8,7 @@
 
 #define HASH_PROBE_1(__k, ok, i)                                                                   \
     ok = false;                                                                                    \
-    i = (uintptr_t)(__k) & self->mask;                                                             \
+    i = (uintptr_t)(__k)&self->mask;                                                               \
     while(self->items[i].key != NULL) {                                                            \
         if(self->items[i].key == (__k)) {                                                          \
             ok = true;                                                                             \
@@ -101,18 +101,24 @@ bool NameDict__del(NameDict* self, py_Name key) {
     self->items[i].key = NULL;
     self->items[i].value = *py_NIL();
     self->length--;
-    // tidy
-    uintptr_t pre_z = i;
-    uintptr_t z = (i + 1) & self->mask;
-    while(self->items[z].key != NULL) {
-        uintptr_t h = (uintptr_t)self->items[z].key & self->mask;
-        if(h != i) break;
-        // std::swap(_items[pre_z], _items[z]);
-        NameDict_KV tmp = self->items[pre_z];
-        self->items[pre_z] = self->items[z];
-        self->items[z] = tmp;
-        pre_z = z;
-        z = (z + 1) & self->mask;
+    /* tidy */
+    uint32_t posToRemove = i;
+    uint32_t posToShift = posToRemove;
+    while(true) {
+        posToShift = (posToShift + 1) & self->mask;
+        if(self->items[posToShift].key == NULL) break;
+        uintptr_t hash_z = (uintptr_t)self->items[posToShift].key;
+        uintptr_t insertPos = hash_z & self->mask;
+        bool cond1 = insertPos <= posToRemove;
+        bool cond2 = posToRemove <= posToShift;
+        if((cond1 && cond2) ||
+           // chain wrapped around capacity
+           (posToShift < insertPos && (cond1 || cond2))) {
+            NameDict_KV tmp = self->items[posToRemove];
+            self->items[posToRemove] = self->items[posToShift];
+            self->items[posToShift] = tmp;
+            posToRemove = posToShift;
+        }
     }
     return true;
 }

src/public/py_dict.c

@@ -5,6 +5,16 @@
 #include "pocketpy/interpreter/types.h"
 #include "pocketpy/interpreter/vm.h"
 
+typedef struct {
+    Dict* dict;  // weakref for slot 0
+    Dict dict_backup;
+    DictEntry* curr;
+    DictEntry* end;
+    int mode;  // 0: keys, 1: values, 2: items
+} DictIterator;
+
+#define Dict__step(x) ((x) < mask ? (x) + 1 : 0)
+
 static uint32_t Dict__next_cap(uint32_t cap) {
     switch(cap) {
         case 7: return 17;
@@ -51,19 +61,36 @@ static uint32_t Dict__next_cap(uint32_t cap) {
     }
 }
 
-
-
-typedef struct {
-    DictEntry* curr;
-    DictEntry* end;
-    int mode;  // 0: keys, 1: values, 2: items
-} DictIterator;
+static uint64_t Dict__hash(uint64_t key) {
+    // https://gist.github.com/badboy/6267743
+    key = (~key) + (key << 21);  // key = (key << 21) - key - 1
+    key = key ^ (key >> 24);
+    key = (key + (key << 3)) + (key << 8);  // key * 265
+    key = key ^ (key >> 14);
+    key = (key + (key << 2)) + (key << 4);  // key * 21
+    key = key ^ (key >> 28);
+    key = key + (key << 31);
+    return key;
+}
 
 static void Dict__ctor(Dict* self, uint32_t capacity, int entries_capacity) {
     self->length = 0;
     self->capacity = capacity;
-    self->indices = PK_MALLOC(self->capacity * sizeof(DictIndex));
-    memset(self->indices, -1, self->capacity * sizeof(DictIndex));
+
+    size_t indices_size;
+    if(self->capacity < UINT16_MAX) {
+        self->index_is_short = true;
+        indices_size = self->capacity * sizeof(uint16_t);
+        self->null_index_value = UINT16_MAX;
+    } else {
+        self->index_is_short = false;
+        indices_size = self->capacity * sizeof(uint32_t);
+        self->null_index_value = UINT32_MAX;
+    }
+
+    self->indices = PK_MALLOC(indices_size);
+    memset(self->indices, -1, indices_size);
+
     c11_vector__ctor(&self->entries, sizeof(DictEntry));
     c11_vector__reserve(&self->entries, entries_capacity);
 }
@@ -75,70 +102,114 @@ static void Dict__dtor(Dict* self) {
     c11_vector__dtor(&self->entries);
 }
 
-static bool Dict__try_get(Dict* self, py_TValue* key, DictEntry** out) {
-    py_i64 hash;
-    if(!py_hash(key, &hash)) return false;
-    int idx = (uint64_t)hash % self->capacity;
-    for(int i = 0; i < PK_DICT_MAX_COLLISION; i++) {
-        int idx2 = self->indices[idx]._[i];
-        if(idx2 == -1) continue;
+static uint32_t Dict__get_index(Dict* self, uint32_t index) {
+    if(self->index_is_short) {
+        uint16_t* indices = self->indices;
+        return indices[index];
+    } else {
+        uint32_t* indices = self->indices;
+        return indices[index];
+    }
+}
+
+static void Dict__swap_null_index(Dict* self, uint32_t pre_z, uint32_t z) {
+    if(self->index_is_short) {
+        uint16_t* indices = self->indices;
+        assert(indices[pre_z] == UINT16_MAX);
+        indices[pre_z] = indices[z];
+        indices[z] = UINT16_MAX;
+    } else {
+        uint32_t* indices = self->indices;
+        assert(indices[pre_z] == UINT32_MAX);
+        indices[pre_z] = indices[z];
+        indices[z] = UINT32_MAX;
+    }
+}
+
+static void Dict__set_index(Dict* self, uint32_t index, uint32_t value) {
+    if(self->index_is_short) {
+        uint16_t* indices = self->indices;
+        indices[index] = (uint16_t)value;
+    } else {
+        uint32_t* indices = self->indices;
+        indices[index] = value;
+    }
+}
+
+static bool Dict__probe(Dict* self,
+                        py_TValue* key,
+                        uint64_t* p_hash,
+                        uint32_t* p_idx,
+                        DictEntry** p_entry) {
+    py_i64 h_user;
+    if(!py_hash(key, &h_user)) return false;
+    *p_hash = Dict__hash((uint64_t)h_user);
+    uint32_t mask = self->capacity - 1;
+    uint32_t idx = (*p_hash) % self->capacity;
+    while(true) {
+        uint32_t idx2 = Dict__get_index(self, idx);
+        if(idx2 == self->null_index_value) break;
         DictEntry* entry = c11__at(DictEntry, &self->entries, idx2);
-        if(entry->hash == (uint64_t)hash) {
+        if(entry->hash == (*p_hash)) {
             int res = py_equal(&entry->key, key);
             if(res == 1) {
-                *out = entry;
+                *p_idx = idx;
+                *p_entry = entry;
                 return true;
             }
             if(res == -1) return false;  // error
         }
+        // try next index
+        idx = Dict__step(idx);
     }
-    *out = NULL;
+    // not found
+    *p_idx = idx;
+    *p_entry = NULL;
     return true;
 }
 
+static bool Dict__try_get(Dict* self, py_TValue* key, DictEntry** out) {
+    uint64_t hash;
+    uint32_t idx;
+    return Dict__probe(self, key, &hash, &idx, out);
+}
+
 static void Dict__clear(Dict* self) {
-    memset(self->indices, -1, self->capacity * sizeof(DictIndex));
+    size_t indices_size = self->index_is_short ? self->capacity * sizeof(uint16_t)
+                                               : self->capacity * sizeof(uint32_t);
+    memset(self->indices, -1, indices_size);
     c11_vector__clear(&self->entries);
     self->length = 0;
 }
 
 static void Dict__rehash_2x(Dict* self) {
     Dict old_dict = *self;
-    uint32_t new_capacity = self->capacity;
-
-__RETRY:
-    // use next capacity
-    new_capacity = Dict__next_cap(new_capacity);
+    uint32_t new_capacity = Dict__next_cap(old_dict.capacity);
+    uint32_t mask = new_capacity - 1;
     // create a new dict with new capacity
     Dict__ctor(self, new_capacity, old_dict.entries.capacity);
     // move entries from old dict to new dict
     for(int i = 0; i < old_dict.entries.length; i++) {
         DictEntry* old_entry = c11__at(DictEntry, &old_dict.entries, i);
-        if(py_isnil(&old_entry->key)) continue;
-        int idx = old_entry->hash % new_capacity;
-        bool success = false;
-        for(int i = 0; i < PK_DICT_MAX_COLLISION; i++) {
-            int idx2 = self->indices[idx]._[i];
-            if(idx2 == -1) {
-                // insert new entry (empty slot)
+        if(py_isnil(&old_entry->key)) continue;  // skip deleted
+        uint32_t idx = old_entry->hash % new_capacity;
+        while(true) {
+            uint32_t idx2 = Dict__get_index(self, idx);
+            if(idx2 == self->null_index_value) {
                 c11_vector__push(DictEntry, &self->entries, *old_entry);
-                self->indices[idx]._[i] = self->entries.length - 1;
+                Dict__set_index(self, idx, self->entries.length - 1);
                 self->length++;
-                success = true;
                 break;
             }
-        }
-        if(!success) {
-            Dict__dtor(self);
-            goto __RETRY;
+            // try next index
+            idx = Dict__step(idx);
         }
     }
-    // done
     Dict__dtor(&old_dict);
 }
 
 static void Dict__compact_entries(Dict* self) {
-    int* mappings = PK_MALLOC(self->entries.length * sizeof(int));
+    uint32_t* mappings = PK_MALLOC(self->entries.length * sizeof(uint32_t));
 
     int n = 0;
     for(int i = 0; i < self->entries.length; i++) {
@@ -153,96 +224,97 @@ static void Dict__compact_entries(Dict* self) {
     }
     self->entries.length = n;
     // update indices
-    for(uint32_t i = 0; i < self->capacity; i++) {
-        for(int j = 0; j < PK_DICT_MAX_COLLISION; j++) {
-            int idx = self->indices[i]._[j];
-            if(idx == -1) continue;
-            self->indices[i]._[j] = mappings[idx];
-        }
+    for(int idx = 0; idx < self->capacity; idx++) {
+        uint32_t idx2 = Dict__get_index(self, idx);
+        if(idx2 == self->null_index_value) continue;
+        Dict__set_index(self, idx, mappings[idx2]);
     }
     PK_FREE(mappings);
 }
 
 static bool Dict__set(Dict* self, py_TValue* key, py_TValue* val) {
-    py_i64 hash;
-    if(!py_hash(key, &hash)) return false;
-    int idx = (uint64_t)hash % self->capacity;
-    int bad_hash_count = 0;
-    for(int i = 0; i < PK_DICT_MAX_COLLISION; i++) {
-        int idx2 = self->indices[idx]._[i];
-        if(idx2 == -1) {
-            // insert new entry
-            DictEntry* new_entry = c11_vector__emplace(&self->entries);
-            new_entry->hash = (uint64_t)hash;
-            new_entry->key = *key;
-            new_entry->val = *val;
-            self->indices[idx]._[i] = self->entries.length - 1;
-            self->length++;
-            return true;
-        }
+    uint64_t hash;
+    uint32_t idx;
+    DictEntry* entry;
+    if(!Dict__probe(self, key, &hash, &idx, &entry)) return false;
+    if(entry) {
         // update existing entry
-        DictEntry* entry = c11__at(DictEntry, &self->entries, idx2);
-        // check if they have the same hash
-        if(entry->hash == (uint64_t)hash) {
-            // check if they are equal
-            int res = py_equal(&entry->key, key);
-            if(res == 1) {
-                entry->val = *val;
-                return true;
-            }
-            if(res == -1) return false;  // error
-            // res == 0
-            bad_hash_count++;
-        }
-    }
-    // no empty slot found
-    if(bad_hash_count == PK_DICT_MAX_COLLISION) {
-        // all `PK_DICT_MAX_COLLISION` slots have the same hash but different keys
-        // we are unable to solve this collision via rehashing
-        return RuntimeError("dict: %d/%d/%d: maximum collision reached (hash=%i)",
-                            self->entries.length,
-                            self->entries.capacity,
-                            self->capacity,
-                            hash);
-    }
-
-    if(self->capacity >= (uint32_t)self->entries.length * 10) {
-        return RuntimeError("dict: %d/%d/%d: minimum load factor reached",
-                            self->entries.length,
-                            self->entries.capacity,
-                            self->capacity);
+        entry->val = *val;
+        return true;
     }
-    Dict__rehash_2x(self);
-    return Dict__set(self, key, val);
+    // insert new entry
+    DictEntry* new_entry = c11_vector__emplace(&self->entries);
+    new_entry->hash = hash;
+    new_entry->key = *key;
+    new_entry->val = *val;
+    Dict__set_index(self, idx, self->entries.length - 1);
+    self->length++;
+    // check if we need to rehash
+    float load_factor = (float)self->length / self->capacity;
+    if(load_factor > 0.572) Dict__rehash_2x(self);
+    return true;
 }
 
 /// Delete an entry from the dict.
 /// -1: error, 0: not found, 1: found and deleted
 static int Dict__pop(Dict* self, py_Ref key) {
-    py_i64 hash;
-    if(!py_hash(key, &hash)) return -1;
-    int idx = (uint64_t)hash % self->capacity;
-    for(int i = 0; i < PK_DICT_MAX_COLLISION; i++) {
-        int idx2 = self->indices[idx]._[i];
-        if(idx2 == -1) continue;
-        DictEntry* entry = c11__at(DictEntry, &self->entries, idx2);
-        if(entry->hash == (uint64_t)hash) {
-            int res = py_equal(&entry->key, key);
-            if(res == 1) {
-                *py_retval() = entry->val;
-                py_newnil(&entry->key);
-                self->indices[idx]._[i] = -1;
-                self->length--;
-                if(self->length < self->entries.length / 2) Dict__compact_entries(self);
-                return 1;
-            }
-            if(res == -1) return -1;  // error
+    // Dict__log_index(self, "before pop");
+    uint64_t hash;
+    uint32_t idx;
+    DictEntry* entry;
+    if(!Dict__probe(self, key, &hash, &idx, &entry)) return -1;
+    if(!entry) return 0;  // not found
+
+    // found the entry, delete and return it
+    py_assign(py_retval(), &entry->val);
+    Dict__set_index(self, idx, self->null_index_value);
+    py_newnil(&entry->key);
+    py_newnil(&entry->val);
+    self->length--;
+
+    /* tidy */
+    // https://github.com/OpenHFT/Chronicle-Map/blob/820573a68471509ffc1b0584454f4a67c0be1b84/src/main/java/net/openhft/chronicle/hash/impl/CompactOffHeapLinearHashTable.java#L156
+    uint32_t mask = self->capacity - 1;
+    uint32_t posToRemove = idx;
+    uint32_t posToShift = posToRemove;
+    // int probe_count = 0;
+    // int swap_count = 0;
+    while(true) {
+        posToShift = Dict__step(posToShift);
+        uint32_t idx_z = Dict__get_index(self, posToShift);
+        if(idx_z == self->null_index_value) break;
+        uint64_t hash_z = c11__at(DictEntry, &self->entries, idx_z)->hash;
+        uint32_t insertPos = (uint64_t)hash_z % self->capacity;
+        // the following condition essentially means circular permutations
+        // of three (r = posToRemove, s = posToShift, i = insertPos)
+        // positions are accepted:
+        // [...i..r...s.] or
+        // [...r..s...i.] or
+        // [...s..i...r.]
+        bool cond1 = insertPos <= posToRemove;
+        bool cond2 = posToRemove <= posToShift;
+        if((cond1 && cond2) ||
+           // chain wrapped around capacity
+           (posToShift < insertPos && (cond1 || cond2))) {
+            Dict__swap_null_index(self, posToRemove, posToShift);
+            posToRemove = posToShift;
+            // swap_count++;
         }
+        // probe_count++;
     }
-    return 0;
+    // printf("Dict__pop: probe_count=%d, swap_count=%d\n", probe_count, swap_count);
+    // compact entries if necessary
+    if(self->entries.length > 16 && (self->length < self->entries.length >> 1)) {
+        Dict__compact_entries(self);  // compact entries
+    }
+    // Dict__log_index(self, "after pop");
+    return 1;
 }
 
 static void DictIterator__ctor(DictIterator* self, Dict* dict, int mode) {
+    assert(mode >= 0 && mode <= 2);
+    self->dict = dict;
+    self->dict_backup = *dict;  // backup the dict
     self->curr = dict->entries.data;
     self->end = self->curr + dict->entries.length;
     self->mode = mode;
@@ -257,18 +329,22 @@ static DictEntry* DictIterator__next(DictIterator* self) {
     return retval;
 }
 
+static bool DictIterator__modified(DictIterator* self) {
+    return memcmp(self->dict, &self->dict_backup, sizeof(Dict)) != 0;
+}
+
 ///////////////////////////////
 static bool dict__new__(int argc, py_Ref argv) {
     py_Type cls = py_totype(argv);
     int slots = cls == tp_dict ? 0 : -1;
     Dict* ud = py_newobject(py_retval(), cls, slots, sizeof(Dict));
-    Dict__ctor(ud, 7, 8);
+    Dict__ctor(ud, 7, 4);
     return true;
 }
 
 void py_newdict(py_OutRef out) {
     Dict* ud = py_newobject(out, tp_dict, 0, sizeof(Dict));
-    Dict__ctor(ud, 7, 8);
+    Dict__ctor(ud, 7, 4);
 }
 
 static bool dict__init__(int argc, py_Ref argv) {
@@ -426,12 +502,17 @@ static bool dict_copy(int argc, py_Ref argv) {
     PY_CHECK_ARGC(1);
     Dict* self = py_touserdata(argv);
     Dict* new_dict = py_newobject(py_retval(), tp_dict, 0, sizeof(Dict));
-    new_dict->capacity = self->capacity;
     new_dict->length = self->length;
+    new_dict->capacity = self->capacity;
+    new_dict->null_index_value = self->null_index_value;
+    new_dict->index_is_short = self->index_is_short;
+    // copy entries
     new_dict->entries = c11_vector__copy(&self->entries);
     // copy indices
-    new_dict->indices = PK_MALLOC(new_dict->capacity * sizeof(DictIndex));
-    memcpy(new_dict->indices, self->indices, new_dict->capacity * sizeof(DictIndex));
+    size_t indices_size = self->index_is_short ? self->capacity * sizeof(uint16_t)
+                                               : self->capacity * sizeof(uint32_t);
+    new_dict->indices = PK_MALLOC(indices_size);
+    memcpy(new_dict->indices, self->indices, indices_size);
     return true;
 }
 
@@ -528,6 +609,7 @@ py_Type pk_dict__register() {
 static bool dict_items__next__(int argc, py_Ref argv) {
     PY_CHECK_ARGC(1);
     DictIterator* iter = py_touserdata(py_arg(0));
+    if(DictIterator__modified(iter)) return RuntimeError("dictionary modified during iteration");
     DictEntry* entry = (DictIterator__next(iter));
     if(entry) {
         switch(iter->mode) {
@@ -641,4 +723,4 @@ bool py_dict_apply(py_Ref self, bool (*f)(py_Ref, py_Ref, void*), void* ctx) {
     return true;
 }
 
-#undef PK_DICT_MAX_COLLISION
+#undef Dict__step

tests/08_dict.py

@@ -115,30 +115,7 @@ assert a.pop(1) == 2
 
 assert a.pop(1, None) is None
 
-n = 2 ** 17
-a = {}
-for i in range(n):
-    a[str(i)] = i
-
-for i in range(n):
-    y = a[str(i)]
-
-for i in range(n):
-    del a[str(i)]
-
-# namedict delete test
-# class A: pass
-# a = A()
-# b = ['0', '1']
-
-# for i in range(len(data)):
-#     z = data[i]
-#     setattr(a, str(z), i)
-#     b.append(z)
-#     if i % 3 == 0:
-#         y = b.pop()
-#         delattr(a, y)
-
+# test getitem
 d = {}
 for i in range(-1000, 1000):
     d[i] = i
@@ -155,3 +132,37 @@ assert list(d) == ['1', 222, '333']
 assert list(d.keys()) == ['1', 222, '333']
 assert list(d.values()) == [1, 2, 3]
 assert list(d.items()) == [('1', 1), (222, 2), ('333', 3)]
+
+# test del
+n = 2 ** 17
+a = {}
+for i in range(n):
+    a[str(i)] = i
+for i in range(n):
+    del a[str(i)]
+assert len(a) == 0
+
+# test del with int keys
+if 0:
+    n = 2 ** 17
+    a = {}
+    for i in range(n):
+        a[i] = i
+    for i in range(n):
+        del a[i]
+    assert len(a) == 0
+
+#######################
+
+# namedict delete test
+class A: pass
+a = A()
+b = ['0', '1']
+
+for i in range(len(data)):
+    z = data[i]
+    setattr(a, str(z), i)
+    b.append(z)
+    if i % 3 == 0:
+        y = b.pop()
+        delattr(a, y)

tests/99_extras.py

@@ -103,9 +103,4 @@ class A:
 
 bad_dict = {A(): 1, A(): 2, A(): 3, A(): 4}
 assert len(bad_dict) == 4
-try:
-    bad_dict[A()] = 5   # error
-    exit(1)
-except RuntimeError as e:
-    assert 'maximum collision reached' in str(e)