Patches item #1507011, was opened at 2006-06-16 01:15 Message generated for change (Comment added) made by illume You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1507011&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Core (C code) Group: Python 2.5 Status: Open Resolution: None Priority: 5 Submitted By: Alexander Belopolsky (belopolsky) Assigned to: Nobody/Anonymous (nobody) Summary: Use a set to keep interned strings Initial Comment: This patch is a proof of concept only. In particular, _PySet_LookString is *not* proposed for addition to set API. Better performance can probably be achieved by implementing interning completely inside setobject.c . ---------------------------------------------------------------------- Comment By: Rene Dudfield (illume) Date: 2006-06-30 06:27 Message: Logged In: YES user_id=2042 The author said that the patch is expected to reduce memory. If this is true, then the patch has a good reason to be applied. Another reason is maintainability. By removing the set like functionality which is already implemented in the setobject. ---------------------------------------------------------------------- Comment By: Martin v. Löwis (loewis) Date: 2006-06-17 06:44 Message: Logged In: YES user_id=21627 I don't think "it's natural to do it" is a convincing object reason to change a running system. A change should address correctness, efficiency, maintainability, functionality (features), legal issues, etc. I can't see any of this in this patch, so I'm -1. ---------------------------------------------------------------------- Comment By: Alexander Belopolsky (belopolsky) Date: 2006-06-17 00:52 Message: Logged In: YES user_id=835142 Rationale: The container of interned strings logically a set. Current implementation that uses a dictionary with the same keys and values is clearly an artificial implementation of a set necessary in earlier versions of Python. With a proper C implementation of sets available, it is natural to use it to store interned strings. Since set and dict objects use the same lookup algorithm, this patch is not expected to affect performance and pybench does not show any difference. Since a large set is using half of the memory of the equivalent dict, this patch is expected to reduce interpretor memory usage. ---------------------------------------------------------------------- Comment By: Martin v. Löwis (loewis) Date: 2006-06-16 21:55 Message: Logged In: YES user_id=21627 I did not read the python-dev discussion before writing my first comment, so it is fine that you posted the patch here. Still, I think some rationale should be provided for doing this: what is the advantage? ---------------------------------------------------------------------- Comment By: Alexander Belopolsky (belopolsky) Date: 2006-06-16 12:48 Message: Logged In: YES user_id=835142 The purporse was to allow others to comment on the work in progress. Would it be more appropriate to post it on python-dev instead? If I close a patch while it is not ready, can I reopen it later when it is complete? In any case, please consider interned-set-1.patch for inclusion in Python. ---------------------------------------------------------------------- Comment By: Martin v. Löwis (loewis) Date: 2006-06-16 05:12 Message: Logged In: YES user_id=21627 What is the purpose of posting the patch here, then? The Python patches tracker should only carry patches that are meant for inclusion in Python. If inclusion is not planned, it should be closed (it would still be available, then). ---------------------------------------------------------------------- Comment By: Alexander Belopolsky (belopolsky) Date: 2006-06-16 03:44 Message: Logged In: YES user_id=835142 Fixed code comments and moved all interning logic to setobject.c ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1507011&group_id=5470 _______________________________________________ Patches mailing list [email protected] http://mail.python.org/mailman/listinfo/patches
