Skip to content

Move global statics to module state #207

Description

@tseaver

Closes #207.

This is a preparatory stage toward implementing PEP 489 (multi-phase module initialization + heap-allocated types).

Analysis

Uses of cPersistenceCAPI

By default, the persistent/cPersistent.h header declares a module-static (global across the C module being compiled) pointer named cPersistenceCAPI, which is a pointer to a cPersistenceCAPIstruct holding the C-defined Persistent base class and a set of helper functions (not module methods).

It also defines a big pile of macros (see below) which rely on that static being initalized: the current BTrees modules do so during module initialization: they import the PyCapsule object exported by persistent, and assign it (with a cast) to the static.

Aside from the macros, BTrees code pretty much uses the static only to find the type object for the Persistent base class:

   $ git grep cPersistenceCAPI src/
   src/BTrees/BTreeModuleTemplate.c:    return init_type_with_meta_base(type, &PyType_Type, cPersistenceCAPI->pertype);
   src/BTrees/BTreeModuleTemplate.c:    if (!init_type_with_meta_base(type, &BTreeTypeType, cPersistenceCAPI->pertype)) {
   src/BTrees/BTreeModuleTemplate.c:    cPersistenceCAPI = (cPersistenceCAPIstruct *)PyCapsule_Import(
   src/BTrees/BTreeModuleTemplate.c:    if (cPersistenceCAPI == NULL) {
   src/BTrees/BTreeTemplate.c:    cPersistenceCAPI->pertype->tp_dealloc((PyObject *)self);
   src/BTrees/BTreeTemplate.c:    err = cPersistenceCAPI->pertype->tp_traverse((PyObject *)self, visit, arg);
   src/BTrees/BucketTemplate.c:    cPersistenceCAPI->pertype->tp_dealloc((PyObject *)self);
   src/BTrees/BucketTemplate.c:    err = cPersistenceCAPI->pertype->tp_traverse((PyObject *)self, visit, arg);

Uses of PER_* macros

PER_TypeCheck

Unused in BTrees code

PER_USE_OR_RETURN

   #define PER_USE_OR_RETURN(O,R) {if((O)->state==cPersistent_GHOST_STATE && cPersistenceCAPI->setstate((PyObject*)(O)) < 0) return (R); else if ((O)->state==cPersistent_UPTODATE_STATE) (O)->state=cPersistent_STICKY_STATE;}

Unghostifies a ghost, returning failure code; ensures object is in "sticky" state on success.

   $ git grep PER_USE_OR_RETURN src/
   src/BTrees/BTreeItemsTemplate.c:    PER_USE_OR_RETURN(b, -1);
   src/BTrees/BTreeItemsTemplate.c:        PER_USE_OR_RETURN(b, -1);
   src/BTrees/BTreeItemsTemplate.c:        PER_USE_OR_RETURN(currentbucket, -1);
   src/BTrees/BTreeItemsTemplate.c:        PER_USE_OR_RETURN(currentbucket, -1);
   src/BTrees/BTreeItemsTemplate.c:    PER_USE_OR_RETURN(currentbucket, -1);
   src/BTrees/BTreeItemsTemplate.c:    PER_USE_OR_RETURN(self->currentbucket, NULL);
   src/BTrees/BTreeItemsTemplate.c:    PER_USE_OR_RETURN(bucket, NULL);
   src/BTrees/BTreeModuleTemplate.c:#define PER_USE_OR_RETURN(self, NULL)
   src/BTrees/BTreeModuleTemplate.c: * PER_UNUSE can be used after a successul PER_USE or PER_USE_OR_RETURN.
   src/BTrees/BTreeModuleTemplate.c:        PER_USE_OR_RETURN(first, -1);
   src/BTrees/BTreeTemplate.c:    PER_USE_OR_RETURN(self, -1);
   src/BTrees/BTreeTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BTreeTemplate.c:                PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BTreeTemplate.c:        PER_USE_OR_RETURN(child, -1);
   src/BTrees/BTreeTemplate.c:        PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BTreeTemplate.c:    PER_USE_OR_RETURN(self, -1);
   src/BTrees/BTreeTemplate.c:    /* We don't need to: PER_USE_OR_RETURN(self, -1);
   src/BTrees/BTreeTemplate.c:            PER_USE_OR_RETURN(self, -1);
   src/BTrees/BTreeTemplate.c:        PER_USE_OR_RETURN(bucket, NULL);
   src/BTrees/BTreeTemplate.c:    PER_USE_OR_RETURN(self, -1);
   src/BTrees/BTreeTemplate.c:        PER_USE_OR_RETURN(b, -1);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, -1);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/BucketTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/SetTemplate.c:    PER_USE_OR_RETURN(self, -1);
   src/BTrees/SetTemplate.c:    PER_USE_OR_RETURN(self, NULL);
   src/BTrees/_fsBTree.c:  PER_USE_OR_RETURN(self, NULL);

PER_CHANGED

   #define PER_CHANGED(O) (cPersistenceCAPI->changed((cPersistentObject*)(O)))

Invokes the CAPI changed function, which returns -1 for errors.

   $ git grep PER_CHANGED src/
   src/BTrees/BTreeTemplate.c:    return PER_CHANGED(self) >= 0 ? 0 : -1;
   src/BTrees/BTreeTemplate.c: * CAUTION:  The caller must call PER_CHANGED on self.
   src/BTrees/BTreeTemplate.c:            if (PER_CHANGED(self) < 0)
   src/BTrees/BTreeTemplate.c:        if (PER_CHANGED(self) < 0)
   src/BTrees/BTreeTemplate.c:        if (PER_CHANGED(self) < 0)
   src/BTrees/BucketTemplate.c:            if (PER_CHANGED(self) >= 0)
   src/BTrees/BucketTemplate.c:        if (PER_CHANGED(self) >= 0)
   src/BTrees/BucketTemplate.c:    if (PER_CHANGED(self) >= 0)
   src/BTrees/BucketTemplate.c:    if (PER_CHANGED(self) < 0)
   src/BTrees/BucketTemplate.c:        if (PER_CHANGED(self) < 0)
   src/BTrees/BucketTemplate.c:        if (PER_CHANGED(self) < 0)

PER_READCURRENT

   #define PER_READCURRENT(O, E)                                     \
     if (cPersistenceCAPI->readCurrent((cPersistentObject*)(O)) < 0) { E; }
 $ git grep PER_READCURRENT src/
 src/BTrees/BTreeTemplate.c:    PER_READCURRENT(self, goto Error);

PER_GHOSTIFY

   #define PER_GHOSTIFY(O) (cPersistenceCAPI->ghostify((cPersistentObject*)(O)))

Invokes the CAPI ghostify function, which cannot return an error (ugh).

   $ git grep PER_GHOSTIFY src/
   src/BTrees/BTreeTemplate.c:            PER_GHOSTIFY(self);
   src/BTrees/BucketTemplate.c:            PER_GHOSTIFY(self);

PER_ALLOW_DEACTIVATION

`` c
#define PER_ALLOW_DEACTIVATION(O) ((O)->state==cPersistent_STICKY_STATE && ((O)->state=cPersistent_UPTODATE_STATE))


If the object is sticky, make it non-sticky, so that it can be ghostified.  No error can be returned.

```bash
   $ git grep PER_ALLOW_DEACTIVATION src/
   src/BTrees/BTreeModuleTemplate.c:#define PER_ALLOW_DEACTIVATION(self)
   src/BTrees/BTreeModuleTemplate.c:    PER_ALLOW_DEACTIVATION(OBJ);        \
   src/BTrees/BTreeTemplate.c:        PER_ALLOW_DEACTIVATION(child);
   src/BTrees/BTreeTemplate.c:                PER_ALLOW_DEACTIVATION(child2);
   src/BTrees/BTreeTemplate.c:            PER_ALLOW_DEACTIVATION(child);
   src/BTrees/BTreeTemplate.c:    PER_ALLOW_DEACTIVATION(self);
   src/BTrees/BTreeTemplate.c:        PER_ALLOW_DEACTIVATION(activated_child);
   src/BTrees/BTreeTemplate.c:        PER_ALLOW_DEACTIVATION(v);
   src/BTrees/BTreeTemplate.c:    PER_ALLOW_DEACTIVATION(self);
   src/BTrees/BucketTemplate.c:        PER_ALLOW_DEACTIVATION(BUCKET(i->set));
   src/BTrees/SetTemplate.c:      PER_ALLOW_DEACTIVATION(BUCKET(i->set));

PER_PREVENT_DEACTIVATION

   #define PER_PREVENT_DEACTIVATION(O)  ((O)->state==cPersistent_UPTODATE_STATE && ((O)->state=cPersistent_STICKY_STATE))

If the object is up-to-date, make it sticky, so that it cannot be ghostified. No error can be returned.

   $ git grep PER_PREVENT_DEACTIVATION src/
   src/BTrees/BTreeModuleTemplate.c:#define PER_PREVENT_DEACTIVATION(self)
   src/BTrees/BTreeTemplate.c:    PER_PREVENT_DEACTIVATION(self);
   src/BTrees/BucketTemplate.c:    PER_PREVENT_DEACTIVATION(self);
   src/BTrees/SetTemplate.c:    PER_PREVENT_DEACTIVATION(self);
   src/BTrees/TreeSetTemplate.c:    PER_PREVENT_DEACTIVATION(self);

PER_USE

   #define PER_USE(O) \
    (((O)->state != cPersistent_GHOST_STATE \
      || (cPersistenceCAPI->setstate((PyObject*)(O)) >= 0)) \
     ? (((O)->state==cPersistent_UPTODATE_STATE) \
        ? ((O)->state=cPersistent_STICKY_STATE) : 1) : 0)

Make a persistent object usable from C by:

  • Making sure it is not a ghost
  • Making it sticky.
    If you call this and don't call PER_ALLOW_DEACTIVATION, your object will not be ghostified.
    Returns a 1 on success and 0 failure, where failure means error.
   $ git grep "PER_USE\>" src/
   src/BTrees/BTreeItemsTemplate.c:            UNLESS(PER_USE(currentbucket))
   src/BTrees/BTreeItemsTemplate.c:            UNLESS(PER_USE(currentbucket))
   src/BTrees/BTreeModuleTemplate.c:#define PER_USE(O) 1
   src/BTrees/BTreeModuleTemplate.c: * PER_UNUSE can be used after a successul PER_USE or PER_USE_OR_RETURN.
   src/BTrees/BTreeTemplate.c:        UNLESS (PER_USE(child))
   src/BTrees/BTreeTemplate.c:                UNLESS (PER_USE(child2))
   src/BTrees/BTreeTemplate.c:            UNLESS (PER_USE(child))
   src/BTrees/BTreeTemplate.c:        UNLESS(PER_USE(v))
   src/BTrees/BTreeTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BTreeTemplate.c:    UNLESS(PER_USE(d->child))
   src/BTrees/BTreeTemplate.c:                UNLESS(PER_USE(d->child))
   src/BTrees/BTreeTemplate.c:            UNLESS(PER_USE(bucket))
   src/BTrees/BTreeTemplate.c:            UNLESS(PER_USE(d->child))
   src/BTrees/BTreeTemplate.c:            UNLESS(PER_USE(d->child))
   src/BTrees/BTreeTemplate.c:    UNLESS (PER_USE(self)) return NULL;
   src/BTrees/BTreeTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BTreeTemplate.c:        UNLESS(PER_USE(pbucket)) goto Done;
   src/BTrees/BTreeTemplate.c:            UNLESS(PER_USE(deepest_smaller))
   src/BTrees/BTreeTemplate.c:        UNLESS(PER_USE(pbucket))
   src/BTrees/BTreeTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BTreeTemplate.c:        UNLESS (PER_USE(bucket))
   src/BTrees/BTreeTemplate.c:        UNLESS (PER_USE(bucket))
   src/BTrees/BTreeTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BTreeTemplate.c:            UNLESS (PER_USE(lowbucket))
   src/BTrees/BTreeTemplate.c:                UNLESS (PER_USE(lowbucket))
   src/BTrees/BTreeTemplate.c:        UNLESS (PER_USE(highbucket))
   src/BTrees/BTreeTemplate.c:                UNLESS (PER_USE(highbucket))
   src/BTrees/BTreeTemplate.c:        UNLESS (PER_USE(lowbucket))
   src/BTrees/BTreeTemplate.c:        UNLESS (PER_USE(highbucket))
   src/BTrees/BTreeTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BucketTemplate.c:    UNLESS (PER_USE(self)) return NULL;
   src/BTrees/BucketTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BucketTemplate.c:        UNLESS (PER_USE(successor))
   src/BTrees/BucketTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BucketTemplate.c:    UNLESS (PER_USE(self))
   src/BTrees/BucketTemplate.c:        UNLESS(PER_USE(BUCKET(i->set)))
   src/BTrees/SetOpTemplate.c:        UNLESS (PER_USE(b)) goto Error;
   src/BTrees/SetTemplate.c:        UNLESS(PER_USE(BUCKET(i->set)))

PER_ACCESSED

   #define PER_ACCESSED(O)  (cPersistenceCAPI->accessed((cPersistentObject*)(O)))

Invokes the CAPI changed function, which can return an error, but that error is always ignored (ugh).

   $ git grep "PER_ACCESSED" src/
   src/BTrees/BTreeModuleTemplate.c:#define PER_ACCESSED(O) 1
   src/BTrees/BTreeModuleTemplate.c:    PER_ACCESSED(OBJ);                  \
   src/BTrees/BTreeModuleTemplate.c:        PER_ACCESSED(trailing);
   src/BTrees/BucketTemplate.c:          PER_ACCESSED(BUCKET(i->set));
   src/BTrees/SetTemplate.c:          PER_ACCESSED(BUCKET(i->set));

PER_UNUSE

Locally defined (not in persistent):

   #define PER_UNUSE(OBJ) do {             \
       PER_ALLOW_DEACTIVATION(OBJ);        \
       PER_ACCESSED(OBJ);                  \
   } while (0)

Since it uses PER_ACCESSED, we need to ensure that capi_struct is in scope everywhere it is used.

   $ git grep PER_UNUSE
   src/BTrees/BTreeItemsTemplate.c:        PER_UNUSE(b);
   src/BTrees/BTreeItemsTemplate.c:    PER_UNUSE(b);
   src/BTrees/BTreeItemsTemplate.c:        PER_UNUSE(currentbucket);
   src/BTrees/BTreeItemsTemplate.c:        PER_UNUSE(currentbucket);
   src/BTrees/BTreeItemsTemplate.c:    PER_UNUSE(currentbucket);
   src/BTrees/BTreeItemsTemplate.c:    PER_UNUSE(self->currentbucket);
   src/BTrees/BTreeItemsTemplate.c:            PER_UNUSE(currentbucket);
   src/BTrees/BTreeItemsTemplate.c:            PER_UNUSE(currentbucket);
   src/BTrees/BTreeItemsTemplate.c:    PER_UNUSE(bucket);
   src/BTrees/BTreeModuleTemplate.c: * PER_UNUSE can be used after a successul PER_USE or PER_USE_OR_RETURN.
   src/BTrees/BTreeModuleTemplate.c:#define PER_UNUSE(OBJ) do {             \
   src/BTrees/BTreeTemplate.c:                PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(child);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(d->child);
   src/BTrees/BTreeTemplate.c:                PER_UNUSE(d->child);
   src/BTrees/BTreeTemplate.c:            PER_UNUSE(bucket);
   src/BTrees/BTreeTemplate.c:            PER_UNUSE(d->child);
   src/BTrees/BTreeTemplate.c:            PER_UNUSE(d->child);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:                PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(pbucket);
   src/BTrees/BTreeTemplate.c:            PER_UNUSE(deepest_smaller);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(pbucket);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(bucket);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(bucket);
   src/BTrees/BTreeTemplate.c:            PER_UNUSE(lowbucket);
   src/BTrees/BTreeTemplate.c:                PER_UNUSE(lowbucket);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(highbucket);
   src/BTrees/BTreeTemplate.c:                PER_UNUSE(highbucket);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(lowbucket);
   src/BTrees/BTreeTemplate.c:        PER_UNUSE(highbucket);
   src/BTrees/BTreeTemplate.c:    PER_UNUSE(self);

Implementation Plan

Cleanups:

  • Unwind the Py2 compatibility macros, just because they get in the way. (see refactor: scrub Python2 compatibility fossils #206).
  • Ditch use of the cPersistenceCAPI static object, by replacing it with a module state variable.
    • Add a static inline helper function, _get_per_capi, which initially just returns the global static cPersistenceCAPI pointer.
    • Re-wire all functions which use any of the macros above which depend on the static cPersistenceCAPI object to first call the new
      _get_per_capi helper, assigning the result to a locally-declared cPersistenceCAPI.
    • Add a new module_state struct definition, and allocate space for it using the .m_size slot on the PyModule_Def declaration.
    • Move the post-PyModule_Create bits to a new module_exec function.
    • In that function, add an import of the CAPI capsule, and populate the module state struct using that pointer.
    • Add module_traverse and module_clear slot implementations, to ensure that our capsule import plays nice with GC.
    • Update _get_per_capi, such that it navigates to the module, initially using PyState_FindModule), fetches its state using PyModule_GetState, and returns the CAPI pointer.
    • Update all call-sites for the macros to ensure that they find their functions capi_struct pointer using _get_capi_struct.
    • Copy in the macro definitions from cPersistent.h, and update them to use the local capi_struct pointer.
    • Set the #define which suppresses declaring the global static CAPI pointer when including persistent/persistent/cPersistence.h.
      Likewise other global statics:
  • BTreeType_setattro_allowed_names (declared / used in BTreeTemplate.c but initialized in BTreeModuleTemplate.c.
  • object_ (declared / used in objectkeymacros.h, but initialized in BTreeModuleTemplate.c. Remove it altogether: it is a weird hack around not using &PyBaseObject_Type (perhaps for BBB with Python2?).
  • ConflictError (imported from the Python BTrees.intefaces module, used in MergeTemplate.c (which falls back to aliasing ValueError if the static is never initialized).

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions