The slowdown of text-mode pickle is due to the extremely expensive way of unpickling pickled strings in text-mode: it invokes eval() (well, PyRun_String()) to parse the string literal! (After checking that there's really only a string literal there to prevent trojan horses.)
After re-reading the quoted thread, there was another phenomenon remarked upon there: the slow text-mode pickle used less memory. I noticed this too when I ran the test program. The explanation is that the strings in the test program were "key0", "key1", ... "key24" and "value0" ... "value24", over and over (each test dict has the same keys and values). Because these literals look like identifiers, they are interned, so the unpickled data structure shares the string references -- while the original test data has 10,000 copies of each string! If we really want this as a feature, a call to PyString_InternFromString() could be made under certain conditions in load_short_binstring() (e.g. when the length is at most 10 and all_name_chars() from compile.c returns true). I'm not sure that this is a desirable feature though. --Guido van Rossum (home page: http://www.python.org/~guido/)
[Guido]
... Because these literals look like identifiers, they are interned, so the unpickled data structure shares the string references -- while the original test data has 10,000 copies of each string!
If we really want this as a feature, a call to PyString_InternFromString() could be made under certain conditions in load_short_binstring() (e.g. when the length is at most 10 and all_name_chars() from compile.c returns true).
I'm not sure that this is a desirable feature though.
I hope Oren resumes his crusade to make interned strings follow the same refcount rules as everything else, and then we wouldn't have this fear of interning. BTW, nobody yet has reported any code where "indirect interning" pays -- or even triggers once in a non-eating-its-own-tail way.
I hope Oren resumes his crusade to make interned strings follow the same refcount rules as everything else, and then we wouldn't have this fear of interning. BTW, nobody yet has reported any code where "indirect interning" pays -- or even triggers once in a non-eating-its-own-tail way.
Maybe we should just drop indirect interning then. It can save 31 bits per string object, right? How to collect those savings? --Guido van Rossum (home page: http://www.python.org/~guido/)
On Fri, Aug 09, 2002 at 10:14:06AM -0400, Guido van Rossum wrote:
I hope Oren resumes his crusade to make interned strings follow the same refcount rules as everything else, and then we wouldn't have this fear of interning. BTW, nobody yet has reported any code where "indirect interning" pays -- or even triggers once in a non-eating-its-own-tail way.
Maybe we should just drop indirect interning then. It can save 31 bits per string object, right? How to collect those savings?
I was just going back to that patch. The current savings are 24 bits (so now you see why I considered making 'interned' a type - to get that bit in without paying for a whole byte :-). Before the nitpickers point it out: yes, the average savings are likely to be less than 24 bits because of allocator overhead and nonuniform distribution of string lengths. Oren
[Guido]
Maybe we should just drop indirect interning then. It can save 31 bits per string object, right? How to collect those savings?
Make the flag a byte insted of a pointer and it will save 3 or 7 bytes (depending on native pointer size) "on average". Note, assuming a 32-bit box: since pymalloc 8-byte aligns, the smallest footprint a string object can have now is 24 bytes, 20 of which are consumed by bookkeeping overheads (type pointer, refcount, ob_size, ob_shash, ob_sinterned). Strings through length 3 fit in this size (one byte is needed for the trailing \0 we always put in ob_sval[]). Saving 3 bytes wouldn't actually change the memory burden of the smallest string object, but would allow all strings of lengths 4, 5 and 6 to consume 8 fewer bytes than at present (assuming compilers are happy not to pad between a char member and char[] member). That's probably a significant savings for many string-slinging apps (count the number of words of lengths 4, 5 and 6 in this msg (even <wink> benefits <wink>)).
[Guido]
Maybe we should just drop indirect interning then. It can save 31 bits per string object, right? How to collect those savings?
[Tim]
Make the flag a byte insted of a pointer and it will save 3 or 7 bytes (depending on native pointer size) "on average". Note, assuming a 32-bit box: since pymalloc 8-byte aligns, the smallest footprint a string object can have now is 24 bytes, 20 of which are consumed by bookkeeping overheads (type pointer, refcount, ob_size, ob_shash, ob_sinterned). Strings through length 3 fit in this size (one byte is needed for the trailing \0 we always put in ob_sval[]). Saving 3 bytes wouldn't actually change the memory burden of the smallest string object, but would allow all strings of lengths 4, 5 and 6 to consume 8 fewer bytes than at present (assuming compilers are happy not to pad between a char member and char[] member). That's probably a significant savings for many string-slinging apps (count the number of words of lengths 4, 5 and 6 in this msg (even <wink> benefits <wink>)).
This means a change in the string object lay-out, which breaks binary compatibility (the PyString_AS_STRING macro depends on this). I don't mind biting this bullet, but it means we have to increment the API version, and perhaps the warning about API version mismatches should become an error if an extension with too an API version before this change is detected. Oren, how's that patch coming along? :-) --Guido van Rossum (home page: http://www.python.org/~guido/)
On Fri, Aug 09, 2002 at 02:03:10PM -0400, Guido van Rossum wrote:
This means a change in the string object lay-out, which breaks binary compatibility (the PyString_AS_STRING macro depends on this).
I think that making interned string mortal is important enough by itself even without the size reduction. If binary compatibility is important enough it's possible to maintain it.
I don't mind biting this bullet, but it means we have to increment the API version, and perhaps the warning about API version mismatches should become an error if an extension with too an API version before this change is detected.
Oren, how's that patch coming along? :-)
I've just submitted a new patch for http://python.org/sf/576101 It passes regrtest but causes test_gc to leak 20 objects. 13 from test_finalizer_newclass and 7 from test_del_newclass. These leaks go away if test_saveall is skipped. I've tried earlier versions of this patch (which were ok at the time) and they now create this leak too. Some change since the last time I worked on interning must have caused this. Either this change reveals a bug in my patch or my patch reveals a subtle bug in the GC. I don't know why it interacts with GC logic because strings are non-gc objects. I've tried to untrack the interned dictionary because it plays dirty tricks with refcounts but it doesn't change the symptom. Oren
I've just submitted a new patch for http://python.org/sf/576101
I'll review it when I've got time.
It passes regrtest but causes test_gc to leak 20 objects. 13 from test_finalizer_newclass and 7 from test_del_newclass. These leaks go away if test_saveall is skipped. I've tried earlier versions of this patch (which were ok at the time) and they now create this leak too.
Some change since the last time I worked on interning must have caused this. Either this change reveals a bug in my patch or my patch reveals a subtle bug in the GC.
I don't know why it interacts with GC logic because strings are non-gc objects. I've tried to untrack the interned dictionary because it plays dirty tricks with refcounts but it doesn't change the symptom.
I've seen this too! But only when I run the full test suite, not when I run test_gc in isolation. I made a number of small changes to the GC code, I'll have to roll them back one at a time to see which one caused this -- and then look for a solution. :-( --Guido van Rossum (home page: http://www.python.org/~guido/)
It passes regrtest but causes test_gc to leak 20 objects. 13 from test_finalizer_newclass and 7 from test_del_newclass. These leaks go away if test_saveall is skipped. I've tried earlier versions of this patch (which were ok at the time) and they now create this leak too.
Some change since the last time I worked on interning must have caused this. Either this change reveals a bug in my patch or my patch reveals a subtle bug in the GC.
I don't know why it interacts with GC logic because strings are non-gc objects. I've tried to untrack the interned dictionary because it plays dirty tricks with refcounts but it doesn't change the symptom.
I've seen this too! But only when I run the full test suite, not when I run test_gc in isolation. I made a number of small changes to the GC code, I'll have to roll them back one at a time to see which one caused this -- and then look for a solution. :-(
Duh. This warning is only printed when regrtest.py is given the -l option. Oren's first paragraph quoted above is exactly right. But none of the changes to C files made in the last month made any difference... The difference is test_gc.py itself! With a checkout from a month ago, if I change the classes in test_finalizer() and test_del() to be new-style classes, I get the same warnings. Maybe Tim understands the problem now? (Summary: why do I get the Warning below.) $ ./python ../Lib/test/regrtest.py -l test_gc test_gc Warning: test created 20 uncollectable object(s). 1 test OK. $ --Guido van Rossum (home page: http://www.python.org/~guido/)
Guido van Rossum wrote:
$ ./python ../Lib/test/regrtest.py -l test_gc test_gc Warning: test created 20 uncollectable object(s). 1 test OK.
Something weird is going on. This patch fixes test_finalizer_newclass: --- Lib/test/test_gc.py 9 Aug 2002 17:38:16 -0000 1.19 +++ Lib/test/test_gc.py 10 Aug 2002 18:33:47 -0000 @@ -147,6 +147,8 @@ else: raise TestFailed, "didn't find obj in garbage (finalizer)" gc.garbage.remove(obj) + del A, B, obj + gc.collect() # finds 13 objects! I guess there is a reference cycle there that wasn't there before. Could it have something to do with tp_del? Neil
$ ./python ../Lib/test/regrtest.py -l test_gc test_gc Warning: test created 20 uncollectable object(s). 1 test OK.
[Neil S]
Something weird is going on. This patch fixes test_finalizer_newclass:
--- Lib/test/test_gc.py 9 Aug 2002 17:38:16 -0000 1.19 +++ Lib/test/test_gc.py 10 Aug 2002 18:33:47 -0000 @@ -147,6 +147,8 @@ else: raise TestFailed, "didn't find obj in garbage (finalizer)" gc.garbage.remove(obj) + del A, B, obj + gc.collect() # finds 13 objects!
I guess there is a reference cycle there that wasn't there before. Could it have something to do with tp_del?
I don't think so -- a Python of a month old had the same warnings when I added these tests that use new-style class. It's much simpler than this: new-style classes have cyclical references to themselves that must be collected. It so happened that the saveall test was fooled by these. Tim checked in a fix that prevents this. --Guido van Rossum (home page: http://www.python.org/~guido/)
[Guido]
... But none of the changes to C files made in the last month made any difference... The difference is test_gc.py itself! With a checkout from a month ago, if I change the classes in test_finalizer() and test_del() to be new-style classes, I get the same warnings.
Maybe Tim understands the problem now? (Summary: why do I get the Warning below.)
$ ./python ../Lib/test/regrtest.py -l test_gc test_gc Warning: test created 20 uncollectable object(s). 1 test OK. $
I think this was shallow, and checked in a change (to test_saveall()) that I believe fixes it. Update and try again?
Guido:
Maybe we should just drop indirect interning then. It can save 31 bits per string object, right? How to collect those savings?
Store the value of strings <= 3 chars long in there. :-) Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
participants (5)
-
Greg Ewing -
Guido van Rossum -
Neil Schemenauer -
Oren Tirosh -
Tim Peters