[Patches] [ python-Patches-424475 ] Speed-up tp_compare usage

[noreply@sourceforge.net]

Patches item #424475, was opened at 2001-05-16 01:07
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=424475&group_id=5470

Category: Core (C code)
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 3
Submitted By: Martin v. Löwis (loewis)
Assigned to: Guido van Rossum (gvanrossum)
Summary: Speed-up tp_compare usage

Initial Comment:
This patch tries to optimize PyObject_RichCompare for
the common case of objects with equal types which
support tp_compare. It gives a speed-up of roughly 7%
for comparing strings in a loop.

The patch also gives type objects a tp_compare
function, so that they can make use of the improvement.

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

>Comment By: Guido van Rossum (gvanrossum)
Date: 2001-10-18 12:21

Message:
Logged In: YES 
user_id=6380

Closing this; I've opened a separate bug for the 2.3
reminder.

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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-10-16 13:35

Message:
Logged In: YES 
user_id=6380

I've applied the recommended doc change.

Reminder: in 2.3, we should actually check the return value
of tp_compare and reject values outside [-1, 1].

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-08-16 02:58

Message:
Logged In: YES 
user_id=21627

The instance-tp-compare-is-special assumption was introduced
with rich comparisons. Currently, it is nowhere specified
that tp_compare *must* return -1/0/1, so for other types, 2
may well mean "one is larger than the other". In fact, in Py
2.1, string_compare would return

Py_CHARMASK(*a->ob_sval) - Py_CHARMASK(*b->ob_sval)

if the first two letters of the string were different.

It is probably ok to tighten this up, but in a phased
manner: First (in 2.2), document that the return type really
is {-1,0,1}; then (in 2.3) extend the documentation to
cover  +2 as well, and perhaps even -2 (exception).

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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-08-16 01:05

Message:
Logged In: YES 
user_id=6380

Thanks for the quick fix. I'll check it in, because I want
to commit some other changes to the same file.

I still feel uneasy about the PyInstance_Check(). Shouldn't
all types be allowed to return 2 from their tp_compare slot?
(In general, the type/class unification makes me feel uneasy
with *any* PyInstance_Check() special cases.)

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-08-16 00:43

Message:
Logged In: YES 
user_id=21627

It appears that do_cmp does not take into account the 
special calling semantics of tp_compare for instances.
The attached cmp.diff fixes this case.


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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-08-15 13:51

Message:
Logged In: YES 
user_id=6380

I have to reopen this, because I've encountered a bug (I
think).
Take a trivial class:
  class C: pass
and compare two instances of it:
  cmp(C(), C())
In Python 2.1.1 and before, this returned the same as
  cmp(id(C()), id(C()))
but currently it always returns the value 2!

This 2 is supposed to be an internal value that should never
be returned.

I am not 100% sure that it is this patch that's at fault,
but I selectively
rolled the object.c part of this patch back, and then it
started doing the
right thing again.

I'm going to check in a test that verifies the correct
behavior.

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-06-09 00:40

Message:
Logged In: YES 
user_id=21627

Committed as object.c 2.132, typeobject.c 2.17, 
UserList.py 1.17.


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

Comment By: Tim Peters (tim_one)
Date: 2001-06-07 13:19

Message:
Logged In: YES 
user_id=31435

Accepted and assigned back to Martin.  This is too valuable 
to quibble over.  Note that when calling a tp_compare slot, 
this kind of thing:

.	c = (*f)(v, w);
.	if (PyErr_Occurred())

is better spelled:

.	c = (*f)(v, w);
.	if (c < 0 && Py_Err_Occurred())


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

Comment By: Martin v. Löwis (loewis)
Date: 2001-05-21 09:57

Message:
Logged In: YES 
user_id=21627

The revised patch prefers tp_compare over tp_richcompare in
do_cmp if both are available. It also restores
UserList.__cmp__ from deprecation.

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

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