[Zodb-checkins] CVS: Zope/lib/python/ZODB - BaseStorage.py:1.20.4.6 Connection.py:1.72.4.5 DemoStorage.py:1.12.4.6 FileStorage.py:1.95.4.5 POSException.py:1.12.4.8 TimeStamp.c:1.15.58.2 Transaction.py:1.37.4.4 __init__.py:1.13.4.4 cPersistence.c:1.62.8.2 coptimizations.c:1.17.58.3

Chris McDonough chrism@zope.com
Tue, 8 Oct 2002 17:46:26 -0400


Update of /cvs-repository/Zope/lib/python/ZODB
In directory cvs.zope.org:/tmp/cvs-serv17057/lib/python/ZODB

Modified Files:
      Tag: chrism-install-branch
	BaseStorage.py Connection.py DemoStorage.py FileStorage.py 
	POSException.py TimeStamp.c Transaction.py __init__.py 
	cPersistence.c coptimizations.c 
Log Message:
More merges from HEAD.


=== Zope/lib/python/ZODB/BaseStorage.py 1.20.4.5 => 1.20.4.6 ===
--- Zope/lib/python/ZODB/BaseStorage.py:1.20.4.5	Tue Oct  8 14:41:13 2002
+++ Zope/lib/python/ZODB/BaseStorage.py	Tue Oct  8 17:45:54 2002
@@ -17,6 +17,7 @@
 import string
 __version__ = string.split('$Revision$')[-2:][0]
 
+import cPickle
 import ThreadLock, bpthread
 import time, UndoLogCompatible
 import POSException
@@ -108,48 +109,54 @@
     def tpc_abort(self, transaction):
         self._lock_acquire()
         try:
-            if transaction is not self._transaction: return
+            if transaction is not self._transaction:
+                return
             self._abort()
             self._clear_temp()
-            self._transaction=None
+            self._transaction = None
             self._commit_lock_release()
-        finally: self._lock_release()
+        finally:
+            self._lock_release()
 
     def _abort(self):
         """Subclasses should redefine this to supply abort actions"""
         pass
 
     def tpc_begin(self, transaction, tid=None, status=' '):
+        if self._is_read_only:
+            raise POSException.ReadOnlyError()
         self._lock_acquire()
         try:
-            if self._transaction is transaction: return
+            if self._transaction is transaction:
+                return
             self._lock_release()
             self._commit_lock_acquire()
             self._lock_acquire()
-            self._transaction=transaction
+            self._transaction = transaction
             self._clear_temp()
 
-            user=transaction.user
-            desc=transaction.description
-            ext=transaction._extension
-            if ext: ext=dumps(ext,1)
-            else: ext=""
-            self._ude=user, desc, ext
+            user = transaction.user
+            desc = transaction.description
+            ext = transaction._extension
+            if ext:
+                ext = cPickle.dumps(ext, 1)
+            else:
+                ext = ""
+            self._ude = user, desc, ext
 
             if tid is None:
-                t=time.time()
-                t=apply(TimeStamp,(time.gmtime(t)[:5]+(t%60,)))
-                self._ts=t=t.laterThan(self._ts)
-                self._serial=`t`
+                now = time.time()
+                t = TimeStamp(*(time.gmtime(now)[:5] + (now % 60,)))
+                self._ts = t = t.laterThan(self._ts)
+                self._serial = `t`
             else:
-                self._ts=TimeStamp(tid)
-                self._serial=tid
-
-            self._tstatus=status
+                self._ts = TimeStamp(tid)
+                self._serial = tid
 
+            self._tstatus = status
             self._begin(self._serial, user, desc, ext)
-
-        finally: self._lock_release()
+        finally:
+            self._lock_release()
 
     def _begin(self, tid, u, d, e):
         """Subclasses should redefine this to supply transaction start actions.
@@ -159,7 +166,8 @@
     def tpc_vote(self, transaction):
         self._lock_acquire()
         try:
-            if transaction is not self._transaction: return
+            if transaction is not self._transaction:
+                return
             self._vote()
         finally:
             self._lock_release()
@@ -172,16 +180,17 @@
     def tpc_finish(self, transaction, f=None):
         self._lock_acquire()
         try:
-            if transaction is not self._transaction: return
+            if transaction is not self._transaction:
+                return
             try:
-                if f is not None: f()
-
-                u,d,e=self._ude
+                if f is not None:
+                    f()
+                u, d, e = self._ude
                 self._finish(self._serial, u, d, e)
                 self._clear_temp()
             finally:
-                self._ude=None
-                self._transaction=None
+                self._ude = None
+                self._transaction = None
                 self._commit_lock_release()
         finally:
             self._lock_release()


=== Zope/lib/python/ZODB/Connection.py 1.72.4.4 => 1.72.4.5 ===
--- Zope/lib/python/ZODB/Connection.py:1.72.4.4	Tue Oct  8 14:41:13 2002
+++ Zope/lib/python/ZODB/Connection.py	Tue Oct  8 17:45:54 2002
@@ -268,28 +268,44 @@
         self.__onCommitActions.append((method_name, args, kw))
         get_transaction().register(self)
 
+    # NB: commit() is responsible for calling tpc_begin() on the storage.
+    # It uses self._begun to track whether it has been called.  When
+    # self._begun is 0, it has not been called.
+
+    # This arrangement allows us to handle the special case of a
+    # transaction with no modified objects.  It is possible for
+    # registration to be occur unintentionally and for a persistent
+    # object to compensate by making itself as unchanged.  When this
+    # happens, it's possible to commit a transaction with no modified
+    # objects.
+
+    # Since tpc_begin() may raise a ReadOnlyError, don't call it if there
+    # are no objects.  This avoids spurious (?) errors when working with
+    # a read-only storage.
+
     def commit(self, object, transaction):
         if object is self:
+            if not self._begun:
+                self._storage.tpc_begin(transaction)
+                self._begun = 1
+
             # We registered ourself.  Execute a commit action, if any.
             if self.__onCommitActions is not None:
                 method_name, args, kw = self.__onCommitActions.pop(0)
                 apply(getattr(self, method_name), (transaction,) + args, kw)
             return
-        oid=object._p_oid
-        invalid=self._invalid
+        
+        oid = object._p_oid
+        invalid = self._invalid
         if oid is None or object._p_jar is not self:
             # new object
             oid = self.new_oid()
-            object._p_jar=self
-            object._p_oid=oid
+            object._p_jar = self
+            object._p_oid = oid
             self._creating.append(oid)
 
         elif object._p_changed:
-            if (
-                (invalid(oid) and not hasattr(object, '_p_resolveConflict'))
-                or
-                invalid(None)
-                ):
+            if invalid(oid) and not hasattr(object, '_p_resolveConflict'):
                 raise ConflictError(object=object)
             self._invalidating.append(oid)
 
@@ -297,7 +313,11 @@
             # Nothing to do
             return
 
-        stack=[object]
+        if not self._begun:
+            self._storage.tpc_begin(transaction)
+            self._begun = 1
+
+        stack = [object]
 
         # Create a special persistent_id that passes T and the subobject
         # stack along:
@@ -330,7 +350,7 @@
         file=StringIO()
         seek=file.seek
         pickler=Pickler(file,1)
-        pickler.persistent_id=new_persistent_id(self, stack.append)
+        pickler.persistent_id=new_persistent_id(self, stack)
         dbstore=self._storage.store
         file=file.getvalue
         cache=self._cache
@@ -351,12 +371,7 @@
                 self._creating.append(oid)
             else:
                 #XXX We should never get here
-                if (
-                    (invalid(oid) and
-                     not hasattr(object, '_p_resolveConflict'))
-                    or
-                    invalid(None)
-                    ):
+                if invalid(oid) and not hasattr(object, '_p_resolveConflict'):
                     raise ConflictError(object=object)
                 self._invalidating.append(oid)
 
@@ -517,8 +532,7 @@
             # storage to make sure that we don't miss an invaildation
             # notifications between the time we check and the time we
             # read.
-            invalid = self._invalid
-            if invalid(oid) or invalid(None):
+            if self._invalid(oid):
                 if not hasattr(object.__class__, '_p_independent'):
                     get_transaction().register(self)
                     raise ReadConflictError(object=object)
@@ -602,33 +616,33 @@
         if self.__onCommitActions is not None:
             del self.__onCommitActions
         self._storage.tpc_abort(transaction)
-        cache=self._cache
-        cache.invalidate(self._invalidated)
-        cache.invalidate(self._invalidating)
+        self._cache.invalidate(self._invalidated)
+        self._cache.invalidate(self._invalidating)
         self._invalidate_creating()
 
     def tpc_begin(self, transaction, sub=None):
-        if self._invalid(None): # Some nitwit invalidated everything!
-            raise ConflictError("transaction already invalidated")
-        self._invalidating=[]
-        self._creating=[]
+        self._invalidating = []
+        self._creating = []
+        self._begun = 0
 
         if sub:
             # Sub-transaction!
-            _tmp=self._tmp
-            if _tmp is None:
-                _tmp=TmpStore.TmpStore(self._version)
-                self._tmp=self._storage
-                self._storage=_tmp
+            if self._tmp is None:
+                _tmp = TmpStore.TmpStore(self._version)
+                self._tmp = self._storage
+                self._storage = _tmp
                 _tmp.registerDB(self._db, 0)
 
-        self._storage.tpc_begin(transaction)
+            # It's okay to always call tpc_begin() for a sub-transaction
+            # because this isn't the real storage.
+            self._storage.tpc_begin(transaction)
+            self._begun = 1
 
     def tpc_vote(self, transaction):
         if self.__onCommitActions is not None:
             del self.__onCommitActions
         try:
-            vote=self._storage.tpc_vote
+            vote = self._storage.tpc_vote
         except AttributeError:
             return
         s = vote(transaction)


=== Zope/lib/python/ZODB/DemoStorage.py 1.12.4.5 => 1.12.4.6 ===


=== Zope/lib/python/ZODB/FileStorage.py 1.95.4.4 => 1.95.4.5 ===
--- Zope/lib/python/ZODB/FileStorage.py:1.95.4.4	Tue Oct  8 14:41:13 2002
+++ Zope/lib/python/ZODB/FileStorage.py	Tue Oct  8 17:45:54 2002
@@ -569,7 +569,7 @@
             h = self._file.read(34)
             _oid = h[:8]
             if _oid != oid:
-                raise CorruptedData, h
+                raise CorruptedDataError, h
             vlen = unpack(">H", h[-2:])[0]
             if vlen:
                 # If there is a version, find out its name and let
@@ -1088,41 +1088,20 @@
             if self._packt is None:
                 raise UndoError(
                     'Undo is currently disabled for database maintenance.<p>')
-            pos = self._pos
-            r = []
-            i = 0
-            # BAW: Why 39 please?  This makes no sense (see also below).
-            while i < last and pos > 39:
-                self._file.seek(pos - 8)
-                pos = pos - U64(self._file.read(8)) - 8
-                self._file.seek(pos)
-                h = self._file.read(TRANS_HDR_LEN)
-                tid, tl, status, ul, dl, el = struct.unpack(">8s8scHHH", h)
-                if tid < self._packt or status == 'p':
-                    break
-                if status != ' ':
-                    continue
-                d = u = ''
-                if ul:
-                    u = self._file.read(ul)
-                if dl:
-                    d = self._file.read(dl)
-                e = {}
-                if el:
-                    try:
-                        e = loads(read(el))
-                    except:
-                        pass
-                d = {'id': base64.encodestring(tid).rstrip(),
-                     'time': TimeStamp(tid).timeTime(),
-                     'user_name': u,
-                     'description': d}
-                d.update(e)
-                if filter is None or filter(d):
-                    if i >= first:
-                        r.append(d)
-                    i += 1
-            return r
+            us = UndoSearch(self._file, self._pos, self._packt,
+                            first, last, filter)
+            while not us.finished():
+                # Hold lock for batches of 20 searches, so default search
+                # parameters will finish without letting another thread run.
+                for i in range(20):
+                    if us.finished():
+                        break
+                    us.search()
+                # Give another thread a chance, so that a long undoLog()
+                # operation doesn't block all other activity.
+                self._lock_release()
+                self._lock_acquire()
+            return us.results
         finally:
             self._lock_release()
 
@@ -1961,7 +1940,7 @@
     file_size=file.tell()
 
     if file_size:
-        if file_size < start: raise FileStorageFormatError, file.name
+        if file_size < start: raise FileStorageFormatError, name
         seek(0)
         if read(4) != packed_version: raise FileStorageFormatError, name
     else:
@@ -2193,7 +2172,7 @@
             file = open(file, 'rb')
         self._file = file
         if file.read(4) != packed_version:
-            raise FileStorageFormatError, name
+            raise FileStorageFormatError, file.name
         file.seek(0,2)
         self._file_size = file.tell()
         self._pos = 4L
@@ -2245,7 +2224,6 @@
         read=file.read
         pos=self._pos
 
-        LOG("ZODB FS", BLATHER, "next(%d)" % index)
         while 1:
             # Read the transaction record
             seek(pos)
@@ -2296,10 +2274,6 @@
                          self._file.name, pos)
                     break
 
-            if self._stop is not None:
-                LOG("ZODB FS", BLATHER,
-                    ("tid %x > stop %x ? %d" %
-                     (U64(tid), U64(self._stop), tid > self._stop)))
             if self._stop is not None and tid > self._stop:
                 raise IndexError, index
 
@@ -2413,3 +2387,59 @@
     """
     def __init__(self, *args):
         self.oid, self.serial, self.version, self.data = args
+
+class UndoSearch:
+
+    def __init__(self, file, pos, packt, first, last, filter=None):
+        self.file = file
+        self.pos = pos
+        self.packt = packt
+        self.first = first
+        self.last = last
+        self.filter = filter
+        self.i = 0
+        self.results = []
+        self.stop = 0
+
+    def finished(self):
+        """Return True if UndoSearch has found enough records."""
+        # BAW: Why 39 please?  This makes no sense (see also below).
+        return self.i >= self.last or self.pos < 39 or self.stop
+
+    def search(self):
+        """Search for another record."""
+        dict = self._readnext()
+        if dict is not None and (self.filter is None or self.filter(dict)):
+            if self.i >= self.first:
+                self.results.append(dict)
+            self.i += 1
+
+    def _readnext(self):
+        """Read the next record from the storage."""
+        self.file.seek(self.pos - 8)
+        self.pos -= U64(self.file.read(8)) + 8
+        self.file.seek(self.pos)
+        h = self.file.read(TRANS_HDR_LEN)
+        tid, tl, status, ul, dl, el = struct.unpack(">8s8scHHH", h)
+        if tid < self.packt or status == 'p':
+            self.stop = 1
+            return None
+        if status != ' ':
+            return None
+        d = u = ''
+        if ul:
+            u = self.file.read(ul)
+        if dl:
+            d = self.file.read(dl)
+        e = {}
+        if el:
+            try:
+                e = loads(self.file.read(el))
+            except:
+                pass
+        d = {'id': base64.encodestring(tid).rstrip(),
+             'time': TimeStamp(tid).timeTime(),
+             'user_name': u,
+             'description': d}
+        d.update(e)
+        return d


=== Zope/lib/python/ZODB/POSException.py 1.12.4.7 => 1.12.4.8 ===


=== Zope/lib/python/ZODB/TimeStamp.c 1.15.58.1 => 1.15.58.2 ===


=== Zope/lib/python/ZODB/Transaction.py 1.37.4.3 => 1.37.4.4 ===


=== Zope/lib/python/ZODB/__init__.py 1.13.4.3 => 1.13.4.4 ===
--- Zope/lib/python/ZODB/__init__.py:1.13.4.3	Tue Oct  8 14:41:14 2002
+++ Zope/lib/python/ZODB/__init__.py	Tue Oct  8 17:45:55 2002
@@ -11,31 +11,34 @@
 # FOR A PARTICULAR PURPOSE
 #
 ##############################################################################
+
+__version__ = '3.1b2'
+
 import sys
 import cPersistence, Persistence
 from zLOG import register_subsystem
 register_subsystem('ZODB')
 
 # This is lame. Don't look. :(
-sys.modules['cPersistence']=cPersistence
+sys.modules['cPersistence'] = cPersistence
 
-Persistent=cPersistence.Persistent
+Persistent = cPersistence.Persistent
 
 # Install Persistent and PersistentMapping in Persistence
 if not hasattr(Persistence, 'Persistent'):
-    Persistence.Persistent=Persistent
-    Persistent.__module__='Persistence'
-    Persistence.Overridable=cPersistence.Overridable
-    Persistence.Overridable.__module__='Persistence'
+    Persistence.Persistent = Persistent
+    Persistent.__module__ = 'Persistence'
+    Persistence.Overridable = cPersistence.Overridable
+    Persistence.Overridable.__module__ = 'Persistence'
     if not hasattr(Persistence, 'PersistentMapping'):
         import PersistentMapping
-        sys.modules['PersistentMapping']=PersistentMapping
-        sys.modules['BoboPOS']=sys.modules['ZODB']
-        sys.modules['BoboPOS.PersistentMapping']=PersistentMapping
-        PersistentMapping=PersistentMapping.PersistentMapping
+        sys.modules['PersistentMapping'] = PersistentMapping
+        sys.modules['BoboPOS'] = sys.modules['ZODB']
+        sys.modules['BoboPOS.PersistentMapping'] = PersistentMapping
+        PersistentMapping = PersistentMapping.PersistentMapping
         from PersistentMapping import PersistentMapping
-        Persistence.PersistentMapping=PersistentMapping
-        PersistentMapping.__module__='Persistence'
+        Persistence.PersistentMapping = PersistentMapping
+        PersistentMapping.__module__ = 'Persistence'
         del PersistentMapping
 
 del cPersistence


=== Zope/lib/python/ZODB/cPersistence.c 1.62.8.1 => 1.62.8.2 ===


=== Zope/lib/python/ZODB/coptimizations.c 1.17.58.2 => 1.17.58.3 ===
--- Zope/lib/python/ZODB/coptimizations.c:1.17.58.2	Tue Oct  8 14:41:14 2002
+++ Zope/lib/python/ZODB/coptimizations.c	Tue Oct  8 17:45:55 2002
@@ -33,207 +33,232 @@
 
 typedef struct {
   PyObject_HEAD
-  PyObject *jar, *stackup, *new_oid;
+  PyObject *jar, *stack, *new_oid;
 } persistent_id;
 
-staticforward PyTypeObject persistent_idType;
+static PyTypeObject persistent_idType;
 
 static persistent_id *
 newpersistent_id(PyObject *ignored, PyObject *args)
 {
-  persistent_id *self;
-  PyObject *jar, *stackup;
+    persistent_id *self;
+    PyObject *jar, *stack;
 
-  UNLESS (PyArg_ParseTuple(args, "OO", &jar, &stackup)) return NULL;
-  UNLESS(self = PyObject_NEW(persistent_id, &persistent_idType)) return NULL;
-  Py_INCREF(jar);
-  self->jar=jar;
-  Py_INCREF(stackup);
-  self->stackup=stackup;
-  self->new_oid=NULL;
-  return self;
+    if (!PyArg_ParseTuple(args, "OO!", &jar, &PyList_Type, &stack)) 
+	return NULL;
+    self = PyObject_NEW(persistent_id, &persistent_idType);
+    if (!self)
+	return NULL;
+    Py_INCREF(jar);
+    self->jar = jar;
+    Py_INCREF(stack);
+    self->stack = stack;
+    self->new_oid = NULL;
+    return self;
 }
 
-
 static void
 persistent_id_dealloc(persistent_id *self)
 {
-  Py_DECREF(self->jar);
-  Py_DECREF(self->stackup);
-  Py_XDECREF(self->new_oid);
-  PyObject_DEL(self);
+    Py_DECREF(self->jar);
+    Py_DECREF(self->stack);
+    Py_XDECREF(self->new_oid);
+    PyObject_DEL(self);
+}
+
+/* Returns the klass of a persistent object.
+   Returns NULL for other objects.
+*/
+static PyObject *
+get_class(PyObject *object)
+{
+    PyObject *class = NULL;
+
+    if (!PyExtensionClass_Check(object)) {
+	if (PyExtensionInstance_Check(object)) {
+	    class = PyObject_GetAttr(object, py___class__);
+	    if (!class) {
+		PyErr_Clear();
+		return NULL;
+	    }
+	    if (!PyExtensionClass_Check(class) ||
+		!(((PyExtensionClass*)class)->class_flags 
+		  & PERSISTENT_TYPE_FLAG)) {
+		Py_DECREF(class);
+		return NULL;
+	    }
+	}
+	else
+	    return NULL;
+    }
+    return class;
+}
+
+/* Return a two-tuple of the class's module and name.
+ */
+static PyObject *
+get_class_tuple(PyObject *class, PyObject *oid)
+{
+    PyObject *module = NULL, *name = NULL, *tuple;
+
+    module = PyObject_GetAttr(class, py___module__);
+    if (!module)
+	goto err;
+    if (!PyObject_IsTrue(module)) {
+	Py_DECREF(module);
+	/* XXX Handle degenerate 1.x ZClass case. */
+	return oid;
+    }
+
+    name = PyObject_GetAttr(class, py___name__);
+    if (!name)
+	goto err;
+
+    tuple = PyTuple_New(2);
+    if (!tuple)
+	goto err;
+    PyTuple_SET_ITEM(tuple, 0, module);
+    PyTuple_SET_ITEM(tuple, 1, name);
+
+    return tuple;
+ err:
+    Py_XDECREF(module);
+    Py_XDECREF(name);
+    return NULL;
+}
+
+static PyObject *
+set_oid(persistent_id *self, PyObject *object)
+{
+    PyObject *oid;
+
+    if (!self->new_oid) {
+	self->new_oid = PyObject_GetAttr(self->jar, py_new_oid);
+	if (!self->new_oid)
+	    return NULL;
+    }
+    oid = PyObject_CallObject(self->new_oid, NULL);
+    if (!oid)
+	return NULL;
+    if (PyObject_SetAttr(object, py__p_oid, oid) < 0) 
+	goto err;
+    if (PyObject_SetAttr(object, py__p_jar, self->jar) < 0) 
+	goto err;
+    if (PyList_Append(self->stack, object) < 0)
+	goto err;
+    return oid;
+ err:
+    Py_DECREF(oid);
+    return NULL;
 }
 
 static PyObject *
 persistent_id_call(persistent_id *self, PyObject *args, PyObject *kwargs)
 {
-  PyObject *object, *oid, *jar=NULL, *r=NULL, *klass=NULL;
+    PyObject *object, *oid, *klass=NULL;
+    PyObject *t1, *t2;
+    int setjar = 0;
+
+    if (!PyArg_ParseTuple(args, "O", &object))
+	return NULL;
+
+    klass = get_class(object);
+    if (!klass)
+	goto return_none;
+
+    oid = PyObject_GetAttr(object, py__p_oid);
+    if (!oid) {
+	PyErr_Clear();
+	Py_DECREF(klass);
+	goto return_none;
+    }
 
-  /*
-  def persistent_id(object, self=self,stackup=stackup):
-  */
-  UNLESS (PyArg_ParseTuple(args, "O", &object)) return NULL;
-
-  /*
-    if (not hasattr(object, '_p_oid') or
-        type(object) is ClassType): return None
-   */
-
-
-  /* Filter out most objects with low-level test. 
-     Yee ha! 
-     (Also get klass along the way.)
-  */
-  if (! PyExtensionClass_Check(object)) {
-    if (PyExtensionInstance_Check(object))
-      {
-	UNLESS (klass=PyObject_GetAttr(object, py___class__)) 
-	  {
+    if (oid != Py_None) {
+	PyObject *jar = PyObject_GetAttr(object, py__p_jar);
+	if (!jar)
 	    PyErr_Clear();
-	    goto not_persistent;
-	  }
-	UNLESS (
-		PyExtensionClass_Check(klass) &&
-		(((PyExtensionClass*)klass)->class_flags 
-		 & PERSISTENT_TYPE_FLAG)
-		)
-	  goto not_persistent;
-
-      }
-    else 
-      goto not_persistent;
-  }
-
-  UNLESS (oid=PyObject_GetAttr(object, py__p_oid)) 
-    {
-      PyErr_Clear();
-      goto not_persistent;
-    }
-
-  /*
-      if oid is None or object._p_jar is not self:
-   */
-  if (oid != Py_None)
-    {
-      UNLESS (jar=PyObject_GetAttr(object, py__p_jar)) PyErr_Clear();
-      if (jar && jar != Py_None && jar != self->jar)
-	{
-	  PyErr_SetString(InvalidObjectReference, 
-			  "Attempt to store an object from a foreign "
-			  "database connection");
-	  return NULL;
+	else {
+	    if (jar != Py_None && jar != self->jar) {
+		PyErr_SetString(InvalidObjectReference, 
+				"Attempt to store an object from a foreign "
+				"database connection");
+		goto err;
+	    }
+	    /* Ignore the oid of the unknown jar and assign a new one. */
+	    if (jar == Py_None)
+		setjar = 1;
+	    Py_DECREF(jar);
 	}
     }
 
-  if (oid == Py_None || jar != self->jar)
-    {
-      /*
-          oid = self.new_oid()
-          object._p_jar=self
-          object._p_oid=oid
-          stackup(object)
-      */
-      UNLESS (self->new_oid ||
-	      (self->new_oid=PyObject_GetAttr(self->jar, py_new_oid)))
+    if (oid == Py_None || setjar) {
+	Py_DECREF(oid);
+	oid = set_oid(self, object);
+	if (!oid)
 	    goto err;
-      ASSIGN(oid, PyObject_CallObject(self->new_oid, NULL));
-      UNLESS (oid) goto null_oid;
-      if (PyObject_SetAttr(object, py__p_jar, self->jar) < 0) goto err;
-      if (PyObject_SetAttr(object, py__p_oid, oid) < 0) goto err;
-      UNLESS (r=PyTuple_New(1)) goto err;
-      PyTuple_SET_ITEM(r, 0, object);
-      Py_INCREF(object);
-      ASSIGN(r, PyObject_CallObject(self->stackup, r));
-      UNLESS (r) goto err;
-      Py_DECREF(r);
-    }
-
-  /*
-      klass=object.__class__
-
-      if klass is ExtensionKlass: return oid
-  */
-  
-  if (PyExtensionClass_Check(object)) goto return_oid;
-
-  /*
-      if hasattr(klass, '__getinitargs__'): return oid
-  */
-
-  if ((r=PyObject_GetAttr(klass, py___getinitargs__)))
-    {
-      Py_DECREF(r);
-      goto return_oid;
-    }
-  PyErr_Clear();
-
-  /*
-      module=getattr(klass,'__module__','')
-      if module: klass=module, klass.__name__
-      else: return oid # degenerate 1.x ZClass case
-  */
-  UNLESS (jar=PyObject_GetAttr(klass, py___module__)) goto err;
-
-  UNLESS (PyObject_IsTrue(jar)) goto return_oid;
-
-  ASSIGN(klass, PyObject_GetAttr(klass, py___name__));
-  UNLESS (klass) goto err;
-
-  UNLESS (r=PyTuple_New(2)) goto err;
-  PyTuple_SET_ITEM(r, 0, jar);
-  PyTuple_SET_ITEM(r, 1, klass);
-  klass=r;
-  jar=NULL;
-
-  /*      
-      return oid, klass
-  */
-  UNLESS (r=PyTuple_New(2)) goto err;
-  PyTuple_SET_ITEM(r, 0, oid);
-  PyTuple_SET_ITEM(r, 1, klass);
-  return r;
-
-not_persistent:
-  Py_INCREF(Py_None);
-  return Py_None;
-
-err:
-  Py_DECREF(oid);
-  oid=NULL;
-
-null_oid:
-return_oid:
-  Py_XDECREF(jar);
-  Py_XDECREF(klass);
-  return oid;
+    }
+
+    if (PyExtensionClass_Check(object)
+	|| PyObject_HasAttr(klass, py___getinitargs__))
+	goto return_oid;
+
+    t2 = get_class_tuple(klass, oid);
+    if (!t2)
+	goto err;
+    if (t2 == oid) /* pass through ZClass special case */
+	goto return_oid;
+    t1 = PyTuple_New(2);
+    if (!t1) {
+	Py_DECREF(t2);
+	goto err;
+    }
+    /* use borrowed references to oid and t2 */
+    PyTuple_SET_ITEM(t1, 0, oid);
+    PyTuple_SET_ITEM(t1, 1, t2);
+
+    Py_DECREF(klass);
+
+    return t1;
+
+ err:
+    Py_XDECREF(oid);
+    oid = NULL;
+
+ return_oid:
+    Py_XDECREF(klass);
+    return oid;
+
+ return_none:
+    Py_INCREF(Py_None);
+    return Py_None;
 }
 
 
 static PyTypeObject persistent_idType = {
-  PyObject_HEAD_INIT(NULL)
-  0,				/*ob_size*/
-  "persistent_id",			/*tp_name*/
-  sizeof(persistent_id),		/*tp_basicsize*/
-  0,				/*tp_itemsize*/
-  /* methods */
-  (destructor)persistent_id_dealloc,	/*tp_dealloc*/
-  (printfunc)0,	/*tp_print*/
-  (getattrfunc)0,		/*obsolete tp_getattr*/
-  (setattrfunc)0,		/*obsolete tp_setattr*/
-  (cmpfunc)0,	/*tp_compare*/
-  (reprfunc)0,		/*tp_repr*/
-  0,		/*tp_as_number*/
-  0,		/*tp_as_sequence*/
-  0,		/*tp_as_mapping*/
-  (hashfunc)0,		/*tp_hash*/
-  (ternaryfunc)persistent_id_call,	/*tp_call*/
-  (reprfunc)0,		/*tp_str*/
-  (getattrofunc)0,	/*tp_getattro*/
-  (setattrofunc)0,	/*tp_setattro*/
-  
-  /* Space for future expansion */
-  0L,0L,
-  "C implementation of the persistent_id function defined in Connection.py"
+    PyObject_HEAD_INIT(NULL)
+    0,				/*ob_size*/
+    "persistent_id",			/*tp_name*/
+    sizeof(persistent_id),		/*tp_basicsize*/
+    0,				/*tp_itemsize*/
+    /* methods */
+    (destructor)persistent_id_dealloc,	/*tp_dealloc*/
+    (printfunc)0,	/*tp_print*/
+    (getattrfunc)0,		/*obsolete tp_getattr*/
+    (setattrfunc)0,		/*obsolete tp_setattr*/
+    (cmpfunc)0,	/*tp_compare*/
+    (reprfunc)0,		/*tp_repr*/
+    0,		/*tp_as_number*/
+    0,		/*tp_as_sequence*/
+    0,		/*tp_as_mapping*/
+    (hashfunc)0,		/*tp_hash*/
+    (ternaryfunc)persistent_id_call,	/*tp_call*/
+    (reprfunc)0,		/*tp_str*/
+    (getattrofunc)0,	/*tp_getattro*/
+    (setattrofunc)0,	/*tp_setattro*/
+    
+    /* Space for future expansion */
+    0L,0L,
+    "C implementation of the persistent_id function defined in Connection.py"
 };
 
 /* End of code for persistent_id objects */
@@ -243,45 +268,40 @@
 /* List of methods defined in the module */
 
 static struct PyMethodDef Module_Level__methods[] = {
-  {"new_persistent_id", (PyCFunction)newpersistent_id, METH_VARARGS,
-   "new_persistent_id(jar, stackup, new_oid)"
-   " -- get a new persistent_id function"},
-  {NULL, (PyCFunction)NULL, 0, NULL}		/* sentinel */
+    {"new_persistent_id", (PyCFunction)newpersistent_id, METH_VARARGS,
+     "new_persistent_id(jar, stack) -- get a new persistent_id function"},
+    {NULL, NULL}		/* sentinel */
 };
 
 void
 initcoptimizations(void)
 {
-  PyObject *m, *d;
+    PyObject *m, *d;
 
 #define make_string(S) if (! (py_ ## S=PyString_FromString(#S))) return
-  make_string(_p_oid);
-  make_string(_p_jar);
-  make_string(__getinitargs__);
-  make_string(__module__);
-  make_string(__class__);
-  make_string(__name__);
-  make_string(new_oid);
+    make_string(_p_oid);
+    make_string(_p_jar);
+    make_string(__getinitargs__);
+    make_string(__module__);
+    make_string(__class__);
+    make_string(__name__);
+    make_string(new_oid);
 			
-  /* Get InvalidObjectReference error */
-  UNLESS (m=PyString_FromString("POSException")) return;
-  ASSIGN(m, PyImport_Import(m));
-  UNLESS (m) return;
-  ASSIGN(m, PyObject_GetAttrString(m, "InvalidObjectReference"));
-  UNLESS (m) return;
-  InvalidObjectReference=m;
-
-  UNLESS (ExtensionClassImported) return;
-
-  m = Py_InitModule4("coptimizations", Module_Level__methods,
-		     coptimizations_doc_string,
-		     (PyObject*)NULL,PYTHON_API_VERSION);
-  d = PyModule_GetDict(m);
-
-  persistent_idType.ob_type=&PyType_Type;
-  PyDict_SetItemString(d,"persistent_idType", OBJECT(&persistent_idType));
-
-  /* Check for errors */
-  if (PyErr_Occurred())
-    Py_FatalError("can't initialize module coptimizations");
+    /* Get InvalidObjectReference error */
+    UNLESS (m=PyString_FromString("ZODB.POSException")) return;
+    ASSIGN(m, PyImport_Import(m));
+    UNLESS (m) return;
+    ASSIGN(m, PyObject_GetAttrString(m, "InvalidObjectReference"));
+    UNLESS (m) return;
+    InvalidObjectReference=m;
+
+    if (!ExtensionClassImported) 
+	return;
+
+    m = Py_InitModule3("coptimizations", Module_Level__methods,
+		       coptimizations_doc_string);
+    d = PyModule_GetDict(m);
+
+    persistent_idType.ob_type = &PyType_Type;
+    PyDict_SetItemString(d,"persistent_idType", OBJECT(&persistent_idType));
 }