[Patches] [ python-Patches-1507011 ] Use a set to keep interned strings

[SourceForge.net]

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