Changes by Raymond Hettinger raymond.hettin...@gmail.com:
--
resolution: - fixed
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
___
Roundup Robot added the comment:
New changeset 9e3be159d023 by Raymond Hettinger in branch 'default':
Issue #24681: Move the most likely test first in set_add_entry().
https://hg.python.org/cpython/rev/9e3be159d023
--
___
Python tracker
Changes by STINNER Victor victor.stin...@gmail.com:
--
nosy: -haypo
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
___
___
Python-bugs-list
Raymond Hettinger added the comment:
Sets are under no obligation to keep their code synced with dicts and have over
time diverged in a number of ways.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
Changes by Raymond Hettinger raymond.hettin...@gmail.com:
Added file: http://bugs.python.org/file39992/move_likely_first.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
___
Roundup Robot added the comment:
New changeset bc80c783c4ab by Raymond Hettinger in branch 'default':
Issue #24681: Move the store of so-table to the code block where it is used.
https://hg.python.org/cpython/rev/bc80c783c4ab
--
nosy: +python-dev
Changes by Raymond Hettinger raymond.hettin...@gmail.com:
Added file: http://bugs.python.org/file39993/move_likely_first.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
___
Serhiy Storchaka added the comment:
The patch makes set_add_entry() inconsistent not only with dict, but with other
set methods that use set_lookkey(). I don't know if there is some subtle bug
here, but I feel myself slightly uncomfortable with it. Also the new code looks
a little less
Raymond Hettinger added the comment:
Serhiy, do you see anything actually wrong with the patch? If it isn't broken
in some clear cut way, I'm going to apply it shortly. The comparison with dict
internals is a red herring -- there is no promise need or precedent for making
that code exactly
Changes by Raymond Hettinger raymond.hettin...@gmail.com:
Removed file: http://bugs.python.org/file39992/move_likely_first.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
___
Serhiy Storchaka added the comment:
The condition that is determined by the PyObject_RichCompareBool() check is not
that the element is already present, but that the element was present. Even if
we will agree that such behavior change is appropriate, the inconsistency with
dict makes it
Raymond Hettinger added the comment:
FWIW, my approach is to look at the most important code
paths to see if there is any work being done that isn't
essential for the result being computed.
Next, I look at the generated assembly to estimate speed
by counting memory accesses (and whether they
Raymond Hettinger added the comment:
you have to provide a benchmark
Actually, I don't. When making a small series of changes, benchmarking every
step is waste of time and tends to trap you in local minimums and causes you to
overfit to a particular processor, compiler, or benchmark. The
Eric Snow added the comment:
Thanks for the clear explanation, Raymond. The approach you've described is
useful in a number of circumstances. Would you mind publishing (somewhere
outside the tracker; devguide?) the specific steps you take and the tools you
use?
--
nosy: +eric.snow
STINNER Victor added the comment:
Since it looks like an optimization, can you please provide a benchmark?
--
nosy: +haypo
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
___
Raymond Hettinger added the comment:
You can benchmark if you want. I'm looking for a second pair of eyes to
validate the correctness. My goal is to put the tests and assignments in the
most logical order.
--
___
Python tracker
R. David Murray added the comment:
Victor, I'm hearing Raymond say that it isn't really about optimization, but
about the logical organization of the code. I think making things more
*complicated* requires a benchmark justification, but it doesn't look to me
like this change makes things
Serhiy Storchaka added the comment:
The patch changes behavior. With the patch it would be possible that after
successful set.add(), the item will be not contained in the set. And this
behavior is not consistent with dict behavior.
--
___
Python
STINNER Victor added the comment:
You can benchmark if you want.
No, you have to provide a benchmark if you want to modify the Python core to
optimize it. Otherwise, modifying the code is pointless.
It's a general rule in Python to optimize code. We don't accept optimization
changes if it
STINNER Victor added the comment:
For me, optimizing assembler is still an optimization. I give up, I just don't
care of set performances.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24681
Raymond Hettinger added the comment:
If PyObject_RichCompareBool() reports that a key is already present in the set,
then set_add_entry() is under no further obligation to make an insertion. As
long as there is no risk of segfault, there is no more obligation to cater to
lying or
New submission from Raymond Hettinger:
Since the *found_active* exit is like the *found_error* exit in that it makes
no further use of *entry*, it can be moved before the table/entry_key check
whose purpose is to make sure the *entry* pointer is still valid. This change
doesn't apply to
22 matches
Mail list logo
