New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alternative implementation of interning #36836
Comments
An interned string has a flag set indicating that it is Interned strings are no longer immortal. They are New function (actually a macro) PyString_CheckInterned |
Logged In: YES I like the way you consolidated all of the knowledge about Consider adding an example to the docs of an effective use |
Logged In: YES This implementation supports both mortal and immortal interned PyString_InternInPlace creates an immortal interned string for PyString_Intern creates a mortal interned string that is Most places in the interpreter were changed to PyString_Intern This version of the patch, like the previous one, disables Make sure you rebuild everything after applying this patch |
Logged In: YES Oops, forgot to actually attach the patch. Here it is. |
Logged In: YES General cleanup. Better handling of immortal interned It passes regrtest but causes test_gc to leak 20 objects. 13 |
Logged In: YES Here's an update of the patch for current CVS Could you add documentation to Doc/api/concrete.tex for The variables PYTHON_API_VERSION and PYTHON_API_STRING in (The test_gc failures were unrelated; Tim has fixed this I'm tempted to say that except for the API doc issue this is |
Logged In: YES Question for all other reviewers. Why not replace all calls E.g. the call in PyObject_SetAttr() will immortalize all And having the builtin intern() always return an immortal Most of the uses I could find of PyString_InternFromString() |
Logged In: YES Some mutually unrelated comments:
PyString_InternInPlace(&name);
Py_DECREF(name);
return PyString_AS_STRING(name); from getclassname would break: Intern() would return the Selectively replacing them might be a good idea, though. For |
Logged In: YES
Note that the *only* time when
getclassname() is doing something very unsavory We'll have to scrutinize all calls for tricks like
So do you think there's a need for immortal |
Logged In: YES string_dealloc() is a bit optimistic in that it _Py_ReleaseInternedStrings() should use PyDict_ methods, not I note that the whole patch needs to be scrutinized |
Logged In: YES Yes, PyString_InternInPlace is for backward compatibility. My work copy has an option for making strings binary A related patch (static names) provides a possible |
Logged In: YES _Py_ReleaseInternedStrings: it might be that embedded intern creates immortal strings: It might be that an binary compatibility: I'm neutral here. If the API is PyString_InternInPlace: I think it needs to be preserved, |
Logged In: YES Here's a new version (#6) that makes all interned strings I'm very tempted to check this in and see how it goes.
|
Logged In: YES All checked in. I'm uploading the final version of the |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: