[Python-Dev] Alternative implementation of interning

[Guido van Rossum]

> * Probably makes no difference, but it seems oddly asymmetric to fiddle with
>   the interned string's refcount in string_dealloc, call PyObject_DelItem,
>   then not restore the refcount to zero.

That's unnecessary because the next line executed simply frees the
object.  free() doesn't check the refcount.  It's a bit optimistic in
that it doesn't check the DelItem for an error; but that's a separate
issue, and I don't know what it should do when it gets an error at
that point.  Probably call Py_FatalError(); if it wanted to recover,
it would have to call PyErr_Fetch() / PyErr_Restore() around the
DelItem() call, because we're in a dealloc handler here and that
shouldn't change the exception state.

> * Should be Py_DECREF(keys) (not Py_XDECREF(keys)) in
>   _Py_ReleaseInternedStrings.  If you've gotten that far keys can't be
>   NULL.  If you're worried about keys being NULL, you should check it before
>   the for loop (PyMapping_Size() will barf on a NULL arg).

You're right.  Also, I think it should use PyDict_Keys() and
PyDict_Size() -- it knows that interned is a dict so all the hoopla
that PyMapping_Keys() adds is unnecessary.  Maybe the best thing to do
is to remove _Py_ReleaseInternedStrings() and let Barry worry about
how to implement it the next time he wants to use Insure++.

> Also, regarding the name of PyString_InternInPlace, I see now that's the
> original name.  I suggest that name be deprecated in favor of
> PyString_InternImmortal with a macro defined in stringobject.h for
> compatibility.

Yeah, if we keep the immortality feature at all.

BTW, it looks like Oren was careless with error checking in a few
places.  The whole patch needs to checked carefully.

--Guido van Rossum (home page: http://www.python.org/~guido/)