[prev in list] [next in list] [prev in thread] [next in thread] 

List:       python-patches
Subject:    [Patches] [ python-Patches-1053375 ] Decimal speed up
From:       noreply () sourceforge ! net (SourceForge ! net)
Date:       2004-10-27 8:24:22
Message-ID: E1CMhEG-0004na-CQ () sc8-sf-web1 ! sourceforge ! net
[Download RAW message or body]

Patches item #1053375, was opened at 2004-10-24 16:01
Message generated for change (Comment added) made by rhettinger
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1053375&group_id=5470

Category: Library (Lib)
Group: Python 2.4
>Status: Closed
>Resolution: Accepted
Priority: 5
Submitted By: Facundo Batista (facundobatista)
Assigned to: Raymond Hettinger (rhettinger)
Summary: Decimal speed up

Initial Comment:
Raymond:

Some minor modifications: max() instead of veryfing
which is greater and assigning it, and not passing prec
to _fixexponents() (not uses it).

Major change: Not use _WorkRep anymore! Changed
__add__, __mul__ and __divide__ to support it
(including the support functions _normalize() and
_adjust_coefficients). And of course, Decimal.__new__
does not support passing _WorkRep anymore, ;).

One change that may raise controversial: The syntax to
pass a tuple to __new__ now accepts also a number in
the second position. This is because in some places in
the code the result is just a number, and converting it
to a tuple in the code (and not in __new__) made
redundant some checks in __new__.

All the tests pass ok. The new code shows a 12% speed
up in test_decimal.py and 10% in the telco benchmark.

.   Facundo

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

>Comment By: Raymond Hettinger (rhettinger)
Date: 2004-10-27 01:24

Message:
Logged In: YES 
user_id=80475

Nice simplification.
Checked in with minor modifications:
* reduce() replaced with clearer, faster for-loop (see
timings below)
* chain of ifs covered distinct cases, so used if-elif-else
to avoid  unnecessary tests.

See Lib/decimal.py 1.30

----- Timing of for-loop vs reduce() with lambda --------

>>> min(Timer(s1, setup).repeat(9, 1000))
0.039173469216714996
>>> min(Timer(s1, setup).repeat(9, 100000))
3.8988694078010013
>>> min(Timer(s2, setup).repeat(9, 100000))
2.2458371746090293
>>> s1
'cum = reduce(lambda x,y: 10*x+y, val)'
>>> s2
'\ncum = 0\nfor dig in val:\n    cum = cum * 10 + dig\n'
>>> setup
'\nval = [9,6,1,2,3,0,2,4,1,5,7,1,5,6,9]\n'

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

Comment By: Facundo Batista (facundobatista)
Date: 2004-10-26 20:44

Message:
Logged In: YES 
user_id=752496

Commited the previous minor changes. And took another
approach: left _WorkRep but with improvements:

- Optimized the conversion from Decimal's int to _WorkRep's
int (in _WorkRep).
- Changed a comparison in __add__ so there's no more need of
__cmp__, __abs__, and __neg__ in _WorkRep.
- _WorkRep's sign now uses the scheme of Decimal, so I
eliminated a lot of changes from (0,1) to (1,None,-1) and
viceversa.

Now it shows a speedup of 6% in the tests and 4% in the
telco benchmark, but I think that the greater win is in the
readibility of the code (specially with the signs).

I attach the new decimal.py.

.   Facundo

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

Comment By: Raymond Hettinger (rhettinger)
Date: 2004-10-25 23:53

Message:
Logged In: YES 
user_id=80475

I tend to think this should probably not be done. 

On the plus side, there is the simplification of code
(saving approx 80 lines and eliminating an entire class of
objects).  Also, there is the modest speed-up.

On the minus side, it is rather late to be making big
changes to the module.

More importantly, I'm concerned about inlining _WorkRep's
internals.  Nick's use of Python ints as a temporary
representation is currently confined to _WorkRep.  That
choice was not the right way to go in the long term.  While
it saves time for the Python implementation, a C version
would go back to the method of shuffling bcd digits directly
(giving better big O performance and eliminating an
intermodule code dependency). 

The elimination of prec in the _fixexponents call is okay. 
Also, the max() call is fine.

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

Comment By: Facundo Batista (facundobatista)
Date: 2004-10-25 21:56

Message:
Logged In: YES 
user_id=752496

Oops, don't know what happened. Here's the file complete.

.    Facundo

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

Comment By: Raymond Hettinger (rhettinger)
Date: 2004-10-24 19:15

Message:
Logged In: YES 
user_id=80475

It looks like the file did not attach.
Please submit a whole decimal.py instead of a diff.
Thx, Raymond

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

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

[prev in list] [next in list] [prev in thread] [next in thread]