code cleanup and notes

Author: flexatone

automap.c

@@ -137,7 +137,7 @@ static PyObject *NonUniqueError;
 
 
 // The main storage "table" is an array of TableElement
-typedef struct {
+typedef struct TableElement{
     Py_ssize_t keys_pos;
     Py_hash_t hash;
 } TableElement;
@@ -148,7 +148,7 @@ typedef struct {
 # define SCAN 16
 
 
-typedef enum {
+typedef enum KeysArrayType{
     KAT_LIST = 0, // must be falsy
 
     KAT_INT8, // order matters as ranges of size are used in selection
@@ -208,7 +208,7 @@ at_to_kat(int array_t) {
 }
 
 
-typedef struct {
+typedef struct FAMObject{
     PyObject_VAR_HEAD
     Py_ssize_t table_size;
     TableElement *table;    // an array of TableElement structs
@@ -219,7 +219,7 @@ typedef struct {
 } FAMObject;
 
 
-typedef enum {
+typedef enum ViewKind{
     ITEMS,
     KEYS,
     VALUES,
@@ -249,7 +249,7 @@ char_get_end_p(char* p, Py_ssize_t dt_size) {
 
 static inline Py_hash_t
 uint_to_hash(npy_uint64 v) {
-    Py_hash_t hash = (Py_hash_t)(v >> 1); // divide by 2 so it always fits in signed space
+    Py_hash_t hash = (Py_hash_t)(v >> 1); // half unsigned fits in signed
     if (hash == -1) {
         return -2;
     }
@@ -265,6 +265,7 @@ int_to_hash(npy_int64 v) {
     return hash;
 }
 
+
 // This is a adapted from https://github.com/python/cpython/blob/ba65a065cf07a7a9f53be61057a090f7311a5ad7/Python/pyhash.c#L92
 #define HASH_MODULUS (((size_t)1 << 61) - 1)
 #define HASH_BITS 61
@@ -343,9 +344,11 @@ string_to_hash(char *str, Py_ssize_t len) {
 
 static PyObject *int_cache = NULL;
 
+
 // NOTE: this used to be a Py_ssize_t, which can be 32 bits on some machines and might easily overflow with a few very large indices. Using an explicit 64-bit int seems safer
 static npy_int64 key_count_global = 0;
 
+
 // Fill the int_cache up to size_needed with PyObject ints; `size` is not the key_count_global.
 static int
 int_cache_fill(Py_ssize_t size_needed)
@@ -371,6 +374,7 @@ int_cache_fill(Py_ssize_t size_needed)
     return 0;
 }
 
+
 // Given the current key_count_global, remove cache elements only if the key_count is less than the the current size of the int_cache.
 void
 int_cache_remove(Py_ssize_t key_count)
@@ -388,12 +392,10 @@ int_cache_remove(Py_ssize_t key_count)
 //------------------------------------------------------------------------------
 // FrozenAutoMapIterator functions
 
-typedef struct {
+typedef struct FAMIObject {
     PyObject_VAR_HEAD
     FAMObject *fam;
-    PyObject **keys_fi; // fast items ptr
     PyArrayObject* keys_array;
-    PyObject **int_cache_fi; // fast items ptr
     ViewKind kind;
     int reversed;
     Py_ssize_t index; // current index state, mutated in-place
@@ -439,14 +441,14 @@ fami_iternext(FAMIObject *self)
                 return PyTuple_Pack(
                     2,
                     PyArray_ToScalar(PyArray_GETPTR1(self->keys_array, index), self->keys_array),
-                    self->int_cache_fi[index]
+                    PyList_GET_ITEM(int_cache, index)
                 );
             }
             else {
                 return PyTuple_Pack(
                     2,
-                    self->keys_fi[index],
-                    self->int_cache_fi[index]
+                    PyList_GET_ITEM(self->fam->keys, index),
+                    PyList_GET_ITEM(int_cache, index)
                 );
             }
         }
@@ -455,13 +457,13 @@ fami_iternext(FAMIObject *self)
                 return PyArray_ToScalar(PyArray_GETPTR1(self->keys_array, index), self->keys_array);
             }
             else {
-                PyObject* yield = self->keys_fi[index];
+                PyObject* yield = PyList_GET_ITEM(self->fam->keys, index);
                 Py_INCREF(yield);
                 return yield;
             }
         }
         case VALUES: {
-            PyObject *yield = self->int_cache_fi[index];
+            PyObject *yield = PyList_GET_ITEM(int_cache, index);
             Py_INCREF(yield);
             return yield;
         }
@@ -515,15 +517,12 @@ fami_new(FAMObject *fam, ViewKind kind, int reversed)
     }
     Py_INCREF(fam);
     fami->fam = fam;
-    if (!fam->keys_array_type) {
-        fami->keys_fi = PySequence_Fast_ITEMS(fam->keys);
-        fami->keys_array = NULL;
+    if (fam->keys_array_type) {
+        fami->keys_array = (PyArrayObject *)fam->keys;
     }
     else {
-        fami->keys_fi = NULL;
-        fami->keys_array = (PyArrayObject *)fam->keys;
+        fami->keys_array = NULL;
     }
-    fami->int_cache_fi = PySequence_Fast_ITEMS(int_cache);
     fami->kind = kind;
     fami->reversed = reversed;
     fami->index = 0;
@@ -534,7 +533,7 @@ fami_new(FAMObject *fam, ViewKind kind, int reversed)
 // FrozenAutoMapView functions
 
 // A FAMVObject contains a reference to the FAM from which it was derived
-typedef struct {
+typedef struct FAMVObject{
     PyObject_VAR_HEAD
     FAMObject *fam;
     ViewKind kind;
@@ -708,8 +707,8 @@ lookup_hash(FAMObject *self, PyObject *key, Py_hash_t hash)
     Py_hash_t mixin = Py_ABS(hash);
     Py_ssize_t table_pos = hash & mask;
 
-    PyObject **keys_fi = PySequence_Fast_ITEMS(self->keys);
     PyObject *guess = NULL;
+    PyObject *keys = self->keys;
     int result = -1;
     Py_hash_t h = 0;
 
@@ -723,7 +722,7 @@ lookup_hash(FAMObject *self, PyObject *key, Py_hash_t hash)
                 table_pos++;
                 continue;
             }
-            guess = keys_fi[table[table_pos].keys_pos];
+            guess = PyList_GET_ITEM(keys, table[table_pos].keys_pos);
             if (guess == key) { // Hit. Object ID comparison
                 return table_pos;
             }
@@ -923,6 +922,7 @@ lookup_hash_unicode(
     }
 }
 
+
 // Compare a passed char array to stored keys. This does not use any dynamic memory. Returns -1 on error.
 static Py_ssize_t
 lookup_hash_string(
@@ -2030,9 +2030,8 @@ fam_init(PyObject *self, PyObject *args, PyObject *kwargs)
         }
     }
     else {
-        PyObject **keys_fi = PySequence_Fast_ITEMS(keys);
         for (; i < keys_size; i++) {
-            if (insert(fam, keys_fi[i], i, -1)) {
+            if (insert(fam, PyList_GET_ITEM(keys, i), i, -1)) {
                 goto error;
             }
         }

doc/articles/npy-opt.txt

@@ -1,37 +1,55 @@
 
-These changes integrate direct support of NumPy arrays given as keys to `AutoMap`s and `FrozenAutoMap`s. Improvements are made in `AutoMap` initialization, whereby an array is converted to a list using specialized array methods (`tolist()`) when necessary. Improvements are made in `FrozenAutoMap` initialization, whereby an immutable array, when given as keys, is held as a reference without copy to a list; further, hashing and lookup make use of C types, avoiding any creation of PyObjects.
+These changes integrate direct support of NumPy arrays given as keys to `AutoMap`s and `FrozenAutoMap`s, optimizing their usage.
 
-Operation with non-array keys, hash table usage and scanning, and usage of the PyObject integer cache, remains unchanged.
+Improvements are made in `AutoMap` initialization, whereby an array is converted to a list using optimal array methods; that list is then held as the keys.
 
-On initialization (`fam_init`), a `KeysArrayType` enum value is assigned to the `keys_array_type` attribute of `FAMObject`. This is used for branching in all places where divergent behavior is required between keys stored as lists (as was done previously) or as keys stored as typed arrays.
+Improvements are made in `FrozenAutoMap` initialization, whereby an immutable array (for integer, floating point, and flexible dtypes), when given as keys, is held as a reference without copy to a list. Further, hashing and lookup make use of C types, avoiding any management of PyObjects.
 
+For array dtypes not explicitly handled, or for non-array keys, `FrozenAutoMap` operation is unchanged. In all cases, hash table layout and scanning, and management of the PyObject integer cache, are the same as before.
 
-All `FrozenAutoMap` usage of arrays reduces memory usage: no new `PyObject`s are created.
+A key change is that, on initialization (`fam_init`), a `KeysArrayType` enum value is assigned to the `keys_array_type` attribute of `FAMObject`. This is used for branching in all places where divergent behavior is required between keys stored as lists (as was done previously) or as keys stored as typed arrays.
 
+Performance panels compare FAM(L), FAM(A), AM(A), and Dict (`FrozenAutoMap` created from a list, `FrozenAutoMap` created from an array, `AutoMap` created from an array, and a dictionary implementing an `AutoMap` mapping). Key indicators are the performance of instantiation and lookup.
 
+The relevant comparison for StaticFrame usage is between FAM(A) and AM(A), the latter approximating what StaticFrame does presently when creating `FrozenAutoMap`s. (FAM(L) is not available to StaticFrame as AutoMaps are always created from an array, not a list of Python objects.)
 
-General Changes
+Across all supported types, FAM(A) initialization is more than twice as fast as AM(A). FAM(A) lookup performance varies greatly by type, but always out-performs AM(A), in some cases more than twice as fast as AM(A). In all tests, we see signs that out-performance grows with scale.
 
-More usage of `PySequence_Fast_ITEMS` where possible:
-    In `fam_init()` for initialization from lists
-    In `fami` structs for keys and int_cache access during iteration.
+Independent of performance time, All `FrozenAutoMap` usage of arrays reduces memory usage: no new `PyObject`s are created and the passed array simply has reference incremented.
+
+
+
+Key Changes
 
 Split the old `fam_new()` into `fam_new()` and `fam_init()`, implemented `__setstate__()`, `__getstate__()`:
     To support pickling a FAM with a NumPy array, `__setstate__()` must re-set the `writeable` flag of an arary to False.
     To integrate `__setstate__()`, the old `fam_new()` had to be divided into a `fam_new()` and a `fam_init()`.
 
+Based on `keys_array_type`, `fam_init` calls type-specific insert routines, which use type-specific hash routines to add entries to the hash table.
+
+On lookup, type-specific lookup routines are called based on `keys_array_type`. These routines identify PyObjects as PyArray scalars or native PyObject types, extract the appropriate C-type, compute a hash, and use type-specific lookup routines to discover the position in the keys array.
+
+
 Split `copy()` into `copy()` and `copy_to_new()`.
-    Due to splitting `fam_new()`, copy allocation and copy setting needed to split into to methods.
+    Due to now having `fam_new()` and `fam_init()`, copy allocation and copy setting needed to split into two methods. Now, in `fam_init`, if a `FAMType` is identified as the keys argument, `copy_to_new()` is used to transfer values from the argument to the new instance. The `copy()` function remains, now using `fam_new()` and `copy_to_new()`.
 
-Additions to the `fam` struct:
-    Added `key_buffer`: For Unicode arrays, this is a dynamically allocated buffer of size equal to one more than the array's max number of characters. This buffer is given to `PyUnicode_AsUCS4`, which will add a NULL and is why the size of the buffer is one more than max characters. This is only used for a FAM with Unicode array keys; all other usage keeps this as NULL.
+Additions to the `FAMObject` struct:
+    `key_array_type`: A `KeysArrayType` enum specifying a list or array dtype. As a list is assigned zero (and all other array dtypes as non-zero), the value can be used to branch on non-array versus array processing.
 
-Extended property tests for FAMs with arrays
-    Tests initialization of both contiguous and non-contiguous arrays
-    Tests all features tested for non-array-based AMs
+    `keys_size`: As determining size must branch on `keys_array_type`, this attribute is used to track size, avoiding having to go the underly keys container.
+
+    `key_buffer`: For Unicode arrays, this is a dynamically allocated buffer of size equal to one more than the array's max number of characters. This buffer is given to `PyUnicode_AsUCS4`, which will add a NULL and is why the size of the buffer is one more than max characters. This is only used for a FAM with Unicode array keys; all other usage keeps this as NULL.
 
 The type of `key_count_global` is now a platform independent 64 bit signed integer. Perviously, it was a `Py_ssize_t`, which is 32 bits on 32 bit systems and could overflow in scenarios when many indicies are created.
 
+Extended property tests for FAMs with arrays
+    A custom Hypothesis strategy has been implemented to deliver both contiguous and non-contiguous arrays.
+
+    New Hypothesis tests for array-initialized `FrozenAutoMap`s now cover all features previously tested by Hypothesis.
+
+
+
+
 
 
 
@@ -123,50 +141,3 @@ Second Approach
 
     Given PyObjects, can convert to C-types for type-specific loookup.
 
-
-Performance
-
-    NumPy unicode arrays had particularly bad performance
-
-
-Code
-    fam_new() for normal objects
-        insert()
-        lookup_hash()
-
-    fam_new() for int arrays
-        INSERT_SCALARS()
-        insert_int()
-        lookup_hash_int(): use integer as hash!
-
-    fam_new() for unicode arrays
-        insert_unicode()
-        UCS4_to_hash()
-        lookup_hash_unicode()
-
-    fam_new() for AutoMap
-
-
-
-Next Steps
-
-    Can we use datetime46 arrays directly?
-
-
-
-
-
-
-// if (PyArray_IsScalar(key, Byte))
-// npy_byte temp;
-// PyArray_ScalarAsCtype(key, &temp);
-// v = (npy_int64)temp;
-
-
-
-else if (PyArray_IsScalar(key, Half)) {
-    // fprintf(stderr, "got half");
-    // double temp;
-    // PyArray_ScalarAsCtype(key, &temp);
-    // v = (double)temp;
-    v = (double)PyArrayScalar_VAL(key, Half);