[Patches] Resubmit speedup patch for compile.c

Guido van Rossum guido@python.org
Fri, 28 Apr 2000 12:41:40 -0400


> Below find the reworked patch.  I don't completely agree with the
> objections, but, whatever makes you and Guido happy is good with me.
> 
> I can see the objections to changing the semantics of existing
> variables, however I don't understand why the new members need to be
> added at the _end_ of the compiling structure.  To me this makes the
> code less readable because it puts related items further away in the
> source code.  The compiling structure is only used internally, so I
> don't quite see why rearranging it is a problem.  But, whatever, I
> don't want to argue.

I do want to argue! :-)  You're right, those new members don't need to
be banned to the end.

I've also replaced your code to create a tuple with a call to
Py_BuildValue() -- much less code and doesn't seem to be any slower.

Here's the patch that will go in now.

Also thanks for changing the bizarre error handling in com_init() into
something sane!

Index: compile.c
===================================================================
RCS file: /projects/cvsroot/python/dist/src/Python/compile.c,v
retrieving revision 2.105
diff -c -r2.105 compile.c
*** compile.c	2000/04/13 14:10:44	2.105
--- compile.c	2000/04/28 16:39:16
***************
*** 301,307 ****
--- 301,309 ----
  struct compiling {
  	PyObject *c_code;		/* string */
  	PyObject *c_consts;	/* list of objects */
+ 	PyObject *c_const_dict; /* inverse of c_consts */
  	PyObject *c_names;	/* list of strings (names) */
+ 	PyObject *c_name_dict;  /* inverse of c_names */
  	PyObject *c_globals;	/* dictionary (value=None) */
  	PyObject *c_locals;	/* dictionary (value=localID) */
  	PyObject *c_varnames;	/* list (inverse of c_locals) */
***************
*** 403,409 ****
  static void com_addoparg Py_PROTO((struct compiling *, int, int));
  static void com_addfwref Py_PROTO((struct compiling *, int, int *));
  static void com_backpatch Py_PROTO((struct compiling *, int));
! static int com_add Py_PROTO((struct compiling *, PyObject *, PyObject *));
  static int com_addconst Py_PROTO((struct compiling *, PyObject *));
  static int com_addname Py_PROTO((struct compiling *, PyObject *));
  static void com_addopname Py_PROTO((struct compiling *, int, node *));
--- 405,411 ----
  static void com_addoparg Py_PROTO((struct compiling *, int, int));
  static void com_addfwref Py_PROTO((struct compiling *, int, int *));
  static void com_backpatch Py_PROTO((struct compiling *, int));
! static int com_add Py_PROTO((struct compiling *, PyObject *, PyObject *, PyObject *));
  static int com_addconst Py_PROTO((struct compiling *, PyObject *));
  static int com_addname Py_PROTO((struct compiling *, PyObject *));
  static void com_addopname Py_PROTO((struct compiling *, int, node *));
***************
*** 421,442 ****
  	struct compiling *c;
  	char *filename;
  {
  	if ((c->c_code = PyString_FromStringAndSize((char *)NULL,
  						    1000)) == NULL)
! 		goto fail_3;
  	if ((c->c_consts = PyList_New(0)) == NULL)
! 		goto fail_2;
  	if ((c->c_names = PyList_New(0)) == NULL)
! 		goto fail_1;
  	if ((c->c_globals = PyDict_New()) == NULL)
! 		goto fail_0;
  	if ((c->c_locals = PyDict_New()) == NULL)
! 		goto fail_00;
  	if ((c->c_varnames = PyList_New(0)) == NULL)
! 		goto fail_000;
  	if ((c->c_lnotab = PyString_FromStringAndSize((char *)NULL,
  						      1000)) == NULL)
! 		goto fail_0000;
  	c->c_nlocals = 0;
  	c->c_argcount = 0;
  	c->c_flags = 0;
--- 423,449 ----
  	struct compiling *c;
  	char *filename;
  {
+ 	memset((void *)c, '\0', sizeof(struct compiling));
  	if ((c->c_code = PyString_FromStringAndSize((char *)NULL,
  						    1000)) == NULL)
! 		goto fail;
  	if ((c->c_consts = PyList_New(0)) == NULL)
! 		goto fail;
! 	if ((c->c_const_dict = PyDict_New()) == NULL)
! 		goto fail;
  	if ((c->c_names = PyList_New(0)) == NULL)
! 		goto fail;
! 	if ((c->c_name_dict = PyDict_New()) == NULL)
! 		goto fail;
  	if ((c->c_globals = PyDict_New()) == NULL)
! 		goto fail;
  	if ((c->c_locals = PyDict_New()) == NULL)
! 		goto fail;
  	if ((c->c_varnames = PyList_New(0)) == NULL)
! 		goto fail;
  	if ((c->c_lnotab = PyString_FromStringAndSize((char *)NULL,
  						      1000)) == NULL)
! 		goto fail;
  	c->c_nlocals = 0;
  	c->c_argcount = 0;
  	c->c_flags = 0;
***************
*** 458,476 ****
  	c-> c_lnotab_next = 0;
  	return 1;
  	
!   fail_0000:
!   	Py_DECREF(c->c_varnames);
!   fail_000:
!   	Py_DECREF(c->c_locals);
!   fail_00:
!   	Py_DECREF(c->c_globals);
!   fail_0:
!   	Py_DECREF(c->c_names);
!   fail_1:
! 	Py_DECREF(c->c_consts);
!   fail_2:
! 	Py_DECREF(c->c_code);
!   fail_3:
   	return 0;
  }
  
--- 465,472 ----
  	c-> c_lnotab_next = 0;
  	return 1;
  	
!   fail:
! 	com_free(c);
   	return 0;
  }
  
***************
*** 480,486 ****
--- 476,484 ----
  {
  	Py_XDECREF(c->c_code);
  	Py_XDECREF(c->c_consts);
+ 	Py_XDECREF(c->c_const_dict);
  	Py_XDECREF(c->c_names);
+ 	Py_XDECREF(c->c_name_dict);
  	Py_XDECREF(c->c_globals);
  	Py_XDECREF(c->c_locals);
  	Py_XDECREF(c->c_varnames);
***************
*** 665,688 ****
  /* Handle literals and names uniformly */
  
  static int
! com_add(c, list, v)
  	struct compiling *c;
  	PyObject *list;
  	PyObject *v;
  {
! 	int n = PyList_Size(list);
! 	int i;
! 	/* XXX This is quadratic in the number of names per compilation unit.
! 	   XXX Should use a dictionary. */
! 	for (i = n; --i >= 0; ) {
! 		PyObject *w = PyList_GetItem(list, i);
! 		if (v->ob_type == w->ob_type && PyObject_Compare(v, w) == 0)
! 			return i;
! 	}
! 	/* Check for error from PyObject_Compare */
! 	if (PyErr_Occurred() || PyList_Append(list, v) != 0)
! 		c->c_errors++;
  	return n;
  }
  
  static int
--- 663,701 ----
  /* Handle literals and names uniformly */
  
  static int
! com_add(c, list, dict, v)
  	struct compiling *c;
  	PyObject *list;
+ 	PyObject *dict;
  	PyObject *v;
  {
! 	PyObject *w, *t, *np=NULL;
! 	long n;
! 
! 	t = Py_BuildValue("(OO)", v, v->ob_type);
! 	if (t == NULL)
! 	    goto fail;
! 	w = PyDict_GetItem(dict, t);
! 	if (w != NULL) {
! 		n = PyInt_AsLong(w);
! 	} else {
! 		n = PyList_Size(list);
! 		np = PyInt_FromLong(n);
! 		if (np == NULL)
! 		    goto fail;
! 		if (PyList_Append(list, v) != 0)
! 		    goto fail;
! 		if (PyDict_SetItem(dict, t, np) != 0)
! 		    goto fail;
! 		Py_DECREF(np);
! 	}
! 	Py_DECREF(t);
  	return n;
+   fail:
+ 	Py_XDECREF(np);
+ 	Py_XDECREF(t);
+ 	c->c_errors++;
+ 	return 0;
  }
  
  static int
***************
*** 690,696 ****
  	struct compiling *c;
  	PyObject *v;
  {
! 	return com_add(c, c->c_consts, v);
  }
  
  static int
--- 703,709 ----
  	struct compiling *c;
  	PyObject *v;
  {
! 	return com_add(c, c->c_consts, c->c_const_dict, v);
  }
  
  static int
***************
*** 698,704 ****
  	struct compiling *c;
  	PyObject *v;
  {
! 	return com_add(c, c->c_names, v);
  }
  
  #ifdef PRIVATE_NAME_MANGLING
--- 711,717 ----
  	struct compiling *c;
  	PyObject *v;
  {
! 	return com_add(c, c->c_names, c->c_name_dict, v);
  }
  
  #ifdef PRIVATE_NAME_MANGLING

--Guido van Rossum (home page: http://www.python.org/~guido/)