[Patches] [ python-Patches-593627 ] Static names

noreply@sourceforge.net noreply@sourceforge.net
Mon, 12 Aug 2002 05:48:07 -0700


Patches item #593627, was opened at 2002-08-11 12:41
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=593627&group_id=5470

Category: Core (C code)
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Oren Tirosh (orenti)
Assigned to: Nobody/Anonymous (nobody)
Summary: Static names

Initial Comment:
This patch creates static string objects for all built-in 
names and interns then on initialization. The macro 
PyNAME is be used to access static names.

PyNAME(__spam__) is equivalent to 
PyString_InternFromString("__spam__") but is a 
constant expression. It requires the name to be one of 
the built-in names. A linker error will be generated if it 
isn't.

Most conversions of C strings into temporary string 
objects can be eliminated (PyString_FromString, 
PyString_InternFromString). Most string comparisons at 
runtime can also be eliminated. 

Instead of :
if (strcmp(PyString_AsString(name), "__spam__")) ...

This code can be used:
PyString_INTERN(name)
if (name == PyNAME(__spam__)) ...

Where PyString_INTERN is a fast inline check if the 
string is already interned (and it usually is). To prevent 
unbounded accumulation of interned strings the mortal 
interned string patch should also be applied.

The patch converts most of the builtin module to this 
new mode as an example.


----------------------------------------------------------------------

>Comment By: Martin v. Löwis (loewis)
Date: 2002-08-12 14:48

Message:
Logged In: YES 
user_id=21627

It is difficult to see how this patch achieves the goal of
readability:

- many variables are not used at all, e.g. ArithmethicError;
it is not clear how using them would improve readability.

- the change from
 	SETBUILTIN("None",
	Py_None);
   to
 	SETBUILTIN(PyNAME(None),
	Py_None);

   makes it more difficult to read, not easier. Furthermore,
the name "None" isn't used anywhere except this initialisation.

- likewise, the changes from
  	{"abs",
	builtin_abs,        METH_O, abs_doc},
   to
  	{PyNAMEC(abs),
	builtin_abs,        METH_O, abs_doc},

make the code harder to read.


----------------------------------------------------------------------

Comment By: Oren Tirosh (orenti)
Date: 2002-08-12 14:10

Message:
Logged In: YES 
user_id=562624

If these changes are applied throughout the interpreter I 
expect a significant speedup but that is not my immediate 
goal. I am looking for redability, reduction of code size (both 
source and binary) and reliability (less things to check or 
forget to check).

I am trying to get rid of code like

         if (docstr == NULL) {
                docstr= PyString_InternFromString("__doc__");
                if (docstr == NULL)
                        return NULL;
 
And replace it with just PyNAME(__doc__)


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-08-12 09:43

Message:
Logged In: YES 
user_id=21627

What is the rationale for this patch? If it is for
performance, what real speed improvements can you report?

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-11 17:54

Message:
Logged In: YES 
user_id=33168

I'd like to see the releasing interned string patch applied.
 I think it's almost ready, isn't it?  It would make
patching easier and seems to be a good idea.

For me, the easiest way to produce patches is to use cvs. 
You can keep multiple cvs trees around easy enough (for
having multiple overlapping/independant patches).  To create
patches with cvs:

  cvs diff -C 5 [file1] [file2] ....

----------------------------------------------------------------------

Comment By: Oren Tirosh (orenti)
Date: 2002-08-11 17:44

Message:
Logged In: YES 
user_id=562624

Ok, I'll fix the problems with the patch.  What's the best way 
to produce a patch that adds new files? 

Static string objects cannot be released, of course. This 
patch will eventually depend on on the mortal interned strings 
patch to fix it but in the meantime I just disabled releasing 
interned strings because I want to keep the two patches 
independent.

The next version will add a new PyArg_ParseTuple format for 
an interned string to make it easier to use.


----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-11 16:46

Message:
Logged In: YES 
user_id=33168

Couple of initial comments:
 * this is a reverse patch
 * it seems like there are other changes in here
    - int ob_shash -> long
    - releasing interned strings?
 * dictobject.c is removed?
 * including python headers should use "" not <>

Oren, could you generate a new patch with only the changes
to support PyNAME?  Thanks!

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=593627&group_id=5470