Rev 4714: Work on making intern() not generate immortal Key objects. in http://bazaar.launchpad.net/~jameinel/bzr/2.1-memory-consumption

John Arbash Meinel john at arbash-meinel.com
Wed Sep 30 17:50:57 BST 2009


At http://bazaar.launchpad.net/~jameinel/bzr/2.1-memory-consumption

------------------------------------------------------------
revno: 4714
revision-id: john at arbash-meinel.com-20090930165051-538myw1dpmz0214g
parent: john at arbash-meinel.com-20090930045500-ax0xgvu0v6ejtpi9
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1-memory-consumption
timestamp: Wed 2009-09-30 11:50:51 -0500
message:
  Work on making intern() not generate immortal Key objects.
-------------- next part --------------
=== modified file 'bzrlib/_keys_type_c.c'
--- a/bzrlib/_keys_type_c.c	2009-09-30 04:45:53 +0000
+++ b/bzrlib/_keys_type_c.c	2009-09-30 16:50:51 +0000
@@ -36,15 +36,21 @@
 
 
 static PyObject *Keys_item(Keys *self, Py_ssize_t offset);
-static PyObject *key_intern = NULL;
+static PyObject *_interned_keys = NULL;
 
 
 static inline int
-Key_size(Key *self)
+_Key_size(Key *self)
 {
     return self->info & KEY_SIZE_MASK;
 }
 
+static inline int
+_Key_is_interned(Key *self)
+{
+    return self->info & KEY_INTERNED_FLAG;
+}
+
 
 
 #define Key_CheckExact(op) (Py_TYPE(op) == &Key_Type)
@@ -55,7 +61,7 @@
     PyObject *tpl = NULL, *obj = NULL;
     int i, len;
 
-    len = Key_size(self);
+    len = _Key_size(self);
     tpl = PyTuple_New(len);
     if (!tpl) {
         /* Malloc failure */
@@ -77,23 +83,32 @@
 {
     PyObject *unique_key = NULL;
 
-    if (key_intern == NULL) {
-        return self;
-    }
-    unique_key = PyDict_GetItem((PyObject *)key_intern, (PyObject *)self);
+    if (_interned_keys == NULL) {
+        return self;
+    }
+    if (_Key_is_interned(self)) {
+        // Already interned
+        Py_INCREF(self);
+        return self;
+    }
+    unique_key = PyDict_GetItem((PyObject *)_interned_keys, (PyObject *)self);
     if (unique_key) {
         // An entry already existed, return it, instead of self
         Py_INCREF(unique_key);
         return (Key *)unique_key;
     }
     // An entry did not exist, make 'self' the unique item
-    if (PyDict_SetItem(key_intern, (PyObject *)self, (PyObject *)self) < 0) {
+    if (PyDict_SetItem(_interned_keys, (PyObject *)self, (PyObject *)self) < 0) {
         // Suppress an error
         PyErr_Clear();
         return self;
     }
     // self was added to the dict, return it.
     Py_INCREF(self);
+    self->info |= KEY_INTERNED_FLAG;
+    // The two references in the dict do not count, so that the Key object
+    // does not become immortal just because it was interned.
+    Py_REFCNT(self) -= 2;
     return self;
 }
 
@@ -110,7 +125,14 @@
 {
     int i, len;
 
-    len = Key_size(self);
+    if (_Key_is_interned(self)) {
+        /* revive dead object temporarily for DelItem */
+        Py_REFCNT(self) = 3;
+        if (PyDict_DelItem(_interned_keys, (PyObject *)self) != 0) {
+            Py_FatalError("deletion of interned Key failed");
+        }
+    }
+    len = _Key_size(self);
     for (i = 0; i < len; ++i) {
         Py_XDECREF(self->key_bits[i]);
     }
@@ -214,7 +236,7 @@
      * 'stable'?
      */
 	register long x, y;
-	Py_ssize_t len = Key_size(self);
+	Py_ssize_t len = _Key_size(self);
 	PyStringObject **p;
     hashfunc string_hash;
 	long mult = 1000003L;
@@ -329,8 +351,8 @@
     /* It will be rare that we compare tuples of different lengths, so we don't
      * start by optimizing the length comparision, same as the tuple code
      */
-    vlen = Key_size(vk);
-    wlen = Key_size(wk);
+    vlen = _Key_size(vk);
+    wlen = _Key_size(wk);
 	min_len = (vlen < wlen) ? vlen : wlen;
     string_richcompare = PyString_Type.tp_richcompare;
     for (i = 0; i < min_len; i++) {
@@ -401,14 +423,30 @@
 static Py_ssize_t
 Key_length(Key *self)
 {
-    return Key_size(self);
-}
+    return _Key_size(self);
+}
+
+
+static PyObject *
+Key__is_interned(Key *self)
+{
+    if (_Key_is_interned(self)) {
+        Py_INCREF(Py_True);
+        return Py_True;
+    }
+    Py_INCREF(Py_False);
+    return Py_False;
+}
+
+static char Key__is_interned_doc[] = "_is_interned() => True/False\n"
+    "Check to see if this key has been interned.\n";
+
 
 static PyObject *
 Key_item(Key *self, Py_ssize_t offset)
 {
     PyObject *obj;
-    if (offset < 0 || offset >= Key_size(self)) {
+    if (offset < 0 || offset >= _Key_size(self)) {
         PyErr_SetString(PyExc_IndexError, "Key index out of range");
         return NULL;
     }
@@ -450,6 +488,8 @@
 static PyMethodDef Key_methods[] = {
     {"as_tuple", (PyCFunction)Key_as_tuple, METH_NOARGS, Key_as_tuple_doc},
     {"intern", (PyCFunction)Key_intern, METH_NOARGS, Key_intern_doc},
+    {"_is_interned", (PyCFunction)Key__is_interned, METH_NOARGS,
+     Key__is_interned_doc},
     {NULL, NULL} /* sentinel */
 };
 
@@ -956,10 +996,10 @@
     PyModule_AddObject(m, "Keys", (PyObject *)&Keys_Type);
     // Py_INCREF(&KeyIntern_Type);
     // PyModule_AddObject(m, "KeyIntern", (PyObject *)&KeyIntern_Type);
-    // key_intern = PyObject_NewVar(KeyIntern, &KeyIntern_Type, 10);
-    key_intern = PyDict_New();
-    if (key_intern != NULL) {
-        Py_INCREF(key_intern);
-        PyModule_AddObject(m, "_intern", key_intern);
+    // _interned_keys = PyObject_NewVar(KeyIntern, &KeyIntern_Type, 10);
+    _interned_keys = PyDict_New();
+    if (_interned_keys != NULL) {
+        Py_INCREF(_interned_keys);
+        PyModule_AddObject(m, "_interned_keys", _interned_keys);
     }
 }

=== modified file 'bzrlib/_keys_type_py.py'
--- a/bzrlib/_keys_type_py.py	2009-09-29 15:36:59 +0000
+++ b/bzrlib/_keys_type_py.py	2009-09-30 16:50:51 +0000
@@ -62,7 +62,7 @@
         return self._tuple
 
     def intern(self):
-        return _intern.setdefault(self, self)
+        return _interned_keys.setdefault(self, self)
 
 
 
@@ -87,4 +87,4 @@
     return tuple(result)
 
 
-_intern = {}
+_interned_keys = {}

=== modified file 'bzrlib/tests/test__keys_type.py'
--- a/bzrlib/tests/test__keys_type.py	2009-09-30 04:45:53 +0000
+++ b/bzrlib/tests/test__keys_type.py	2009-09-30 16:50:51 +0000
@@ -351,14 +351,14 @@
         unique_str1 = 'unique str ' + osutils.rand_chars(20)
         unique_str2 = 'unique str ' + osutils.rand_chars(20)
         key = self.module.Key(unique_str1, unique_str2)
-        self.assertFalse(key in self.module._intern)
+        self.assertFalse(key in self.module._interned_keys)
         key2 = self.module.Key(unique_str1, unique_str2)
         self.assertEqual(key, key2)
         self.assertIsNot(key, key2)
         key3 = key.intern()
         self.assertIs(key, key3)
-        self.assertTrue(key in self.module._intern)
-        self.assertEqual(key, self.module._intern[key])
+        self.assertTrue(key in self.module._interned_keys)
+        self.assertEqual(key, self.module._interned_keys[key])
         key2 = key2.intern()
         self.assertIs(key, key2)
 
@@ -368,7 +368,8 @@
         unique_str1 = 'unique str ' + osutils.rand_chars(20)
         unique_str2 = 'unique str ' + osutils.rand_chars(20)
         key = self.module.Key(unique_str1, unique_str2)
-        self.assertFalse(key in self.module._intern)
+        self.assertFalse(key in self.module._interned_keys)
+        self.assertFalse(key._is_interned())
         key2 = self.module.Key(unique_str1, unique_str2)
         self.assertEqual(key, key2)
         self.assertIsNot(key, key2)
@@ -377,10 +378,13 @@
 
         key3 = key.intern()
         self.assertIs(key, key3)
-        self.assertTrue(key in self.module._intern)
-        self.assertEqual(key, self.module._intern[key])
+        self.assertTrue(key in self.module._interned_keys)
+        self.assertEqual(key, self.module._interned_keys[key])
         del key3
-        self.assertEqual(4, sys.getrefcount(key))
+        # We should not increase the refcount just via 'intern'
+        self.assertEqual(2, sys.getrefcount(key))
+        self.assertTrue(key._is_interned())
         key2 = key2.intern()
-        self.assertEqual(5, sys.getrefcount(key))
+        # We have one more ref in 'key2' but otherwise no extra refs
+        self.assertEqual(3, sys.getrefcount(key))
         self.assertIs(key, key2)



More information about the bazaar-commits mailing list