This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2012年04月02日 17:16 by Jimbofbx, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| CachingDecimal_example.py | serhiy.storchaka, 2012年04月02日 19:18 | |||
| cachedDecimal.py | Jimbofbx, 2012年04月02日 19:36 | |||
| decimal_hash.diff | rhettinger, 2012年04月07日 17:22 | Cache the decimal hash using a slot | review | |
| decimal_hash_2.patch | serhiy.storchaka, 2012年04月10日 16:10 | review | ||
| Messages (27) | |||
|---|---|---|---|
| msg157369 - (view) | Author: James Hutchison (Jimbofbx) | Date: 2012年04月02日 17:16 | |
Tested on 3.2 Note that I noticed that Decimal is supposed to be faster in 3.3 but I thought I would bring this to light just in case its still relevant Decimal hashing is very slow, even for simple numbers. I found by caching the hash result, there is significant speed up whenever a Decimal value is reused. I created a class that inherits from Decimal and changed the __hash__ function to try/except a stored hash value as such: def __hash__(self): try: return self.myHash; except: self.myHash = super().__hash__(); return self.myHash; Example code: d = dict(); start = time.time(); i = int(1); j = int(2); k = int(3); for i in range(100000): d[i,j,k] = 5; print(time.time() - start); d = dict(); start = time.time(); i = Decimal(1); j = Decimal(2); k = Decimal(3); for i in range(100000): d[i,j,k] = 5; print(time.time() - start); d = dict(); start = time.time(); i = CachingDecimal(1); j = CachingDecimal(2); k = CachingDecimal(3); for i in range(100000): d[i,j,k] = 5; print(time.time() - start); Output int: 0.04699993133544922 Decimal: 2.312999963760376 CachingDecimal: 0.1100001335144043 In a real life example, I changed some of the values in my code from int to Decimal because non-whole numbers needed to be supported (and this seemed like the easiest way to do so without having to worry about my == comparisons breaking) and it slowed my code down massively. Changing to a CachingDecimal type sped up one code block from 92 seconds with Decimal to 7 seconds, which was much closer to the original int speed. Note that all of the relevant operations have to be overloaded to return the CachingDecimal type, which is a pain, so this makes a lot of sense to implement into the Decimal module. My understanding is that altering a Decimal value will always create a new Decimal object, so there shouldn't be any issues with the cached hash desyncing with the correct hash. Someone may want to check that though. Thanks, James |
|||
| msg157375 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2012年04月02日 19:18 | |
It would be better if you provide a working script that demonstrates the issue. I have completed your example (attached). I ran the example on Python 3.1 and received: 0.249999046326 9.8737308979 0.587980985641 Then I ran on Python 3.3: 0.2100839614868164 0.8649246692657471 0.6062228679656982 As you can see, the new implementation is much faster. Benefit from caching decreased. I suppose, if we implement caching in C the difference will be more. Then I increased the size of the cycles in 10 times, and the time is almost equal (on Python 3): 1.8386573791503906 8.418540477752686 8.355770826339722 That I can't to explain. The time of cached version increased disproportionately, more than in 10 times. |
|||
| msg157376 - (view) | Author: James Hutchison (Jimbofbx) | Date: 2012年04月02日 19:36 | |
If I increase the cycles increased 10x with 3.2 I get: int: 0.4219999313354492 Decimal: 24.20299983024597 CachingDecimal: 1.7809998989105225 The sample you have provided is basically what I'm using. See attached What about worst case hash calculation time for Decimal? How does the C code handle that? This is if I increase the values themselves by 100x, same number of loops as above int: 0.5309998989105225 CachingDecimal: 2.078000068664551 Decimal: 41.2979998588562 |
|||
| msg157377 - (view) | Author: James Hutchison (Jimbofbx) | Date: 2012年04月02日 19:38 | |
100x should be e100 |
|||
| msg157446 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2012年04月03日 23:14 | |
I agree that caching the hash would be useful for 3.2, but the
request comes at a unfortunate time: 3.2.3 is about to be released,
and there's no way that the change would go into it.
So let's focus on the C version in 3.3. These are the timings on a
64-bit machine with the C version in 3.3:
int: 0.537806510925293
CachingDecimal: 2.2549374103546143
Decimal: 1.8158345222473145
These are the timings with a hacked C version that caches the hash:
int: 0.5755119323730469
CachingDecimal: 2.3034861087799072
Decimal: 0.4364290237426758
The hash calculation time depends on the size of the coefficient
of the Decimal and the exponent. Note that the context is not
applied when using the Decimal constructor:
>>> Decimal(1e100)
Decimal('10000000000000000159028911097599180468360808563945281389781327557747838772170381060813469985856815104')
So the numbers you are using have an unusually high precision for
regular decimal floating point arithmetic.
If you want well defined limits, I suggest using either:
>>> Decimal('1e100')
Decimal('1E+100')
Or, if the input really must be a float:
>>> c = getcontext()
>>> c.create_decimal(1e100)
Decimal('1.000000000000000015902891110E+100')
In that latter case, of course the conversion is inexact and
rounded (but hashing will be faster).
|
|||
| msg157447 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2012年04月03日 23:29 | |
> but hashing will be faster. I retract that. The exponent actually makes things worse. |
|||
| msg157553 - (view) | Author: Ramchandra Apte (Ramchandra Apte) * | Date: 2012年04月05日 08:13 | |
I recommend that __hash__ should use functools.lru_cache for caching. |
|||
| msg157603 - (view) | Author: James Hutchison (Jimbofbx) | Date: 2012年04月05日 16:33 | |
I presume you mean in 3.2? Have you looked at the source code for that decorator? It's fundamentally a try/except but with a lot more unnecessary bloat than is needed for caching a single int result from a function with no arguments. Its actually a lot slower. If this is likely going to see use in 3.3 then it would probably just be a long int since 3.3 uses C. 0 would indicate uncalculated. Hash function would have to be set up to never return 0. Also every function would need to be tested to make sure there isn't any "in-place" modification of the Decimal object that could alter the hash value. I like how the cached hash in 3.3 is faster than int for hashing. Shouldn't an int just return itself? Why would it be slower? |
|||
| msg157684 - (view) | Author: Jim Jewett (Jim.Jewett) * (Python triager) | Date: 2012年04月06日 20:14 | |
@Jimbofbx: You are correct that a value has to be reserved to indicate that the hash hasn't been (or can't be) computed, but python uses -1 instead of zero. An integer can't return itself because a hash is an unboxed integer; that said, there is a fast path for small enough integers. You can see the actual code at http://hg.python.org/cpython/file/388016438a50/Objects/longobject.c#l2581 Since it is too late for 3.2, I suggest closing this and opening a separate 3.3 issue if the existing improvements are not sufficient. |
|||
| msg157720 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2012年04月07日 10:19 | |
> I recommend that __hash__ should use functools.lru_cache for caching. Why would you do such a thing? A hash value is a single 64-bit slot, no need to add the memory consumption of a whole dictionary and the runtime cost of a LRU eviction policy when you can simply cache the hash in the object itself (like we already do for strings)... |
|||
| msg157731 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2012年04月07日 12:28 | |
> > I recommend that __hash__ should use functools.lru_cache for caching. > Why would you do such a thing? A hash value is a single 64-bit slot, no need to add the memory consumption of a whole dictionary and the runtime cost of a LRU eviction policy when you can simply cache the hash in the object itself (like we already do for strings)... It was a joke (I think). Taking into account the fact that LRU cache uses a hashtable and need to calculate the hash of arguments (i.e., the Decimal self) to get the cached value of hash. |
|||
| msg157732 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2012年04月07日 12:35 | |
> > > I recommend that __hash__ should use functools.lru_cache for caching. > > Why would you do such a thing? A hash value is a single 64-bit slot, no need to add the memory consumption of a whole dictionary and the runtime cost of a LRU eviction policy when you can simply cache the hash in the object itself (like we already do for strings)... > > It was a joke (I think). Taking into account the fact that LRU cache > uses a hashtable and need to calculate the hash of arguments (i.e., the > Decimal self) to get the cached value of hash. Damn. Shame on me for not understanding Raymond's humour :-) |
|||
| msg157741 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2012年04月07日 17:29 | |
Le samedi 07 avril 2012 à 17:22 +0000, Raymond Hettinger a écrit : > ---------- > keywords: +patch > Added file: http://bugs.python.org/file25152/decimal_hash.diff I think patching the C version of Decimal would be more useful :) |
|||
| msg157943 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2012年04月10日 10:26 | |
I think it would be a good idea to fix this in the Python version for the other implementations. |
|||
| msg157955 - (view) | Author: Jim Jewett (Jim.Jewett) * (Python triager) | Date: 2012年04月10日 14:16 | |
Stefan Krah has a good point. Since the only cost is an extra slot, and this is for users who have already chosen to use Decimal instead of a more efficient (but possibly less accurate) representation, even without the native speedups to Decimal ... I guess I'm now +1. It is clearly an implementation detail, so I don't think the docs need to be changed. I'm not sure the tests do, nor am I sure *how* to test for it in a black-box fashion. I have reviewed the patch and have no objections. So unless a new objection comes up in the next few days, I would say this is ready to be checked in. |
|||
| msg157956 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2012年04月10日 14:30 | |
New changeset a012d5df2c73 by Stefan Krah in branch 'default': Issue #14478: Cache the hash of a Decimal in the C version. http://hg.python.org/cpython/rev/a012d5df2c73 |
|||
| msg157957 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2012年04月10日 14:35 | |
I changed the C version to cache the hash as well: For the submitted test case the speedup is only 5x, but hashing times vary greatly depending of the size of the coefficient and the exponent. BTW, the tests already call both hash() and __hash__() on the same number, so retrieving the cached value is actually tested. |
|||
| msg157959 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2012年04月10日 15:15 | |
The patch for the Python version looks good to me. |
|||
| msg157962 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2012年04月10日 16:10 | |
> The patch for the Python version looks good to me Oh, but used by James Hutchison approach is faster. When I disable the _decimal: Unpatched: int: 2.24056077003479 CachingDecimal: 8.49468207359314 Decimal: 187.68132972717285 With rhettinger's LBYL patch: int: 2.1670639514923096 CachingDecimal: 8.790924310684204 Decimal: 10.426796436309814 With EAFP patch: int: 2.1392786502838135 CachingDecimal: 8.431122303009033 Decimal: 8.263015270233154 |
|||
| msg157965 - (view) | Author: James Hutchison (Jimbofbx) | Date: 2012年04月10日 17:00 | |
In the patch: This: + except AttributeError: + pass should be: + except: <everything inside except statement> Checking for the AttributeError is very slightly slower. Not by a lot, but I think if we're going for speed we might as well be as fast as possible. I can't imagine any other exception coming from that try statement. Using except: pass as opposed to sticking everything inside the except statement is also very slightly slower as well Simple test case, 10 million loops: except: 7.140999794006348 except AttributeError: 7.8440001010894775 Exception code in except: 7.483999967575073 after except/pass: 7.75 |
|||
| msg157966 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2012年04月10日 17:14 | |
> I can't imagine any other exception coming from that try statement. KeyboardInterrupt |
|||
| msg157967 - (view) | Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) | Date: 2012年04月10日 17:14 | |
> Checking for the AttributeError is very slightly slower Why are you trying to micro-optimize the Python version, when the C implementation does it better and faster? |
|||
| msg157969 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2012年04月10日 17:29 | |
> Using except: pass as opposed to sticking everything inside the except statement is also very slightly slower as well I ran the test several times and didn't see the difference between pass and sticking variants more than between different runs of the same variant. If it is, then this is a reason for the interpreter optimization. |
|||
| msg157991 - (view) | Author: Raymond Hettinger (rhettinger) * (Python committer) | Date: 2012年04月10日 22:08 | |
I don't see a need to cache the hash value in the pure Python version. |
|||
| msg163602 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2012年06月23日 12:50 | |
The C version of decimal may not always be available. In particular, it is not compatible with C89. Therefore, efficiency of the pure Python version of decimal is important. Any chance to get it in Python 3.3? |
|||
| msg163610 - (view) | Author: Mark Dickinson (mark.dickinson) * (Python committer) | Date: 2012年06月23日 13:48 | |
I agree with Raymond: I don't see a real need to patch the Python version here. If we do want to patch the Python version, I'd go with Raymond's simple patch. |
|||
| msg176355 - (view) | Author: Mark Dickinson (mark.dickinson) * (Python committer) | Date: 2012年11月25日 14:02 | |
Closing as fixed (for the C version, at least). |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:28 | admin | set | github: 58683 |
| 2012年11月25日 14:02:33 | mark.dickinson | set | status: open -> closed resolution: fixed messages: + msg176355 |
| 2012年09月29日 17:48:44 | mark.dickinson | set | versions: + Python 3.4, - Python 3.3 |
| 2012年06月23日 13:48:27 | mark.dickinson | set | messages: + msg163610 |
| 2012年06月23日 13:02:48 | pitrou | set | nosy:
+ mark.dickinson |
| 2012年06月23日 12:50:41 | serhiy.storchaka | set | messages: + msg163602 |
| 2012年04月10日 22:08:58 | rhettinger | set | messages: + msg157991 |
| 2012年04月10日 17:29:18 | serhiy.storchaka | set | messages: + msg157969 |
| 2012年04月10日 17:14:38 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc messages: + msg157967 |
| 2012年04月10日 17:14:32 | serhiy.storchaka | set | messages: + msg157966 |
| 2012年04月10日 17:00:09 | Jimbofbx | set | messages: + msg157965 |
| 2012年04月10日 16:10:39 | serhiy.storchaka | set | files:
+ decimal_hash_2.patch messages: + msg157962 |
| 2012年04月10日 15:15:52 | skrah | set | messages: + msg157959 |
| 2012年04月10日 14:35:01 | skrah | set | messages: + msg157957 |
| 2012年04月10日 14:30:22 | python-dev | set | nosy:
+ python-dev messages: + msg157956 |
| 2012年04月10日 14:16:44 | Jim.Jewett | set | messages: + msg157955 |
| 2012年04月10日 10:26:45 | skrah | set | messages: + msg157943 |
| 2012年04月07日 17:29:34 | pitrou | set | messages: + msg157741 |
| 2012年04月07日 17:22:33 | rhettinger | set | files:
+ decimal_hash.diff keywords: + patch |
| 2012年04月07日 12:35:14 | pitrou | set | messages: + msg157732 |
| 2012年04月07日 12:28:59 | serhiy.storchaka | set | messages: + msg157731 |
| 2012年04月07日 10:20:02 | pitrou | set | stage: needs patch versions: + Python 3.3, - Python 3.2 |
| 2012年04月07日 10:19:55 | pitrou | set | nosy:
+ pitrou messages: + msg157720 |
| 2012年04月07日 03:47:23 | rhettinger | set | assignee: rhettinger nosy: + rhettinger |
| 2012年04月06日 20:14:40 | Jim.Jewett | set | nosy:
+ Jim.Jewett messages: + msg157684 |
| 2012年04月05日 16:33:18 | Jimbofbx | set | messages: + msg157603 |
| 2012年04月05日 08:13:00 | Ramchandra Apte | set | nosy:
+ Ramchandra Apte messages: + msg157553 |
| 2012年04月03日 23:29:41 | skrah | set | messages: + msg157447 |
| 2012年04月03日 23:14:03 | skrah | set | messages: + msg157446 |
| 2012年04月02日 19:38:19 | Jimbofbx | set | messages: + msg157377 |
| 2012年04月02日 19:36:23 | Jimbofbx | set | files:
+ cachedDecimal.py messages: + msg157376 |
| 2012年04月02日 19:18:44 | serhiy.storchaka | set | files:
+ CachingDecimal_example.py nosy: + serhiy.storchaka messages: + msg157375 |
| 2012年04月02日 17:26:38 | jcea | set | nosy:
+ jcea |
| 2012年04月02日 17:22:00 | r.david.murray | set | nosy:
+ skrah |
| 2012年04月02日 17:16:26 | Jimbofbx | create | |