| msg48166 - (view) |
Author: Michael Hudson (mwh)  |
Date: 2005-04-06 13:47 |
| This patch removes longobject.c abuse of ob_size to store the sign in favour of an explicit sign member, as mentioned on python-dev. The only other file that needed modifying is marshal.c (but the marshal format isn't changed). It doesn't touch all the various code that handles ob_size generically. |
|
|
| msg48167 - (view) |
Author: Michael Hudson (mwh) |
Date: 2005-04-06 13:51 |
| Logged In: YES user_id=6656 Oh, and I meant to say, it passes make test but more testing is certainly welcome -- some mistakes show up in pretty obscure ways... |
|
|
| msg48168 - (view) |
Author: Armin Rigo (arigo) * |
Date: 2005-04-08 14:20 |
| Logged In: YES user_id=4771 Unlike Tim I have a use case for lots of small longs in memory: to store unsigned machine integers. It's exactly the case where it would be nice that the extra field didn't cross the malloc 8-byte boundary. Of course, it's exactly NOT what is happening here on 32-bit machines, and this patch makes program relying on this kind of longs suddenly consume 8 extra bytes per long. Damn. |
|
|
| msg48169 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2005-04-10 23:28 |
| Logged In: YES user_id=31435 Armin, I don't understand your use case. Can you be more explicit? Best I can guess, you're using Python longs on a 32-bit box to store positive integers with bit 2**31 set. But there's no change in memory burden for those (rounds up to 24 bytes either way), so that can't be what you mean. Maybe you're using Python longs to store _all_ integers, no matter how small they are? But in that case you weren't serious about memory use to begin with. Michael, I got confused at the start of the patch. When you changed the comment SUM(for i=0 through abs(ob_size)-1) ob_digit[i] * 2**(SHIFT*i) to sign * SUM(for i=0 through ob_size) ob_digit[i] * 2**(SHIFT*i) you dropped the "-1" as well as the "abs()". That wasn't intended, was it? Was also confused by why you dropped the "zero is represented by ob_size == 0." comment. It would be very helpful to rename the new member, e.g., as "long_sign". Then it's easy to find references in the code with an editor search. |
|
|
| msg48170 - (view) |
Author: Michael Hudson (mwh) |
Date: 2005-04-10 23:44 |
| Logged In: YES user_id=6656 Good, I didn't really understand Armin's point either :) Here's a new version of the patch that pays a bit more attention to the comments (I changed my mind over a few details while writing it, I'm not entirely surprised that clarity suffered) and renames the sign member to long_sign -- but it turns out that you could find all references by searching for "->sign"... |
|
|
| msg48171 - (view) |
Author: Armin Rigo (arigo) * |
Date: 2005-04-11 09:38 |
| Logged In: YES user_id=4771 Sorry, I tested the memory overhead of adding an "int" field long_sign, and forgot that the digits were "short". (mwh, your patch #2 forgot to rename "sign" in marshal.c) |
|
|
| msg48172 - (view) |
Author: Armin Rigo (arigo) * |
Date: 2005-04-14 09:34 |
| Logged In: YES user_id=4771 Tim, I don't really have the motivation nor knowledge of the long implementation, so I can't review this patch any better than you did already. Unassigned from me. My general feeling is that mwh+tim+tests is quite safe already :-) |
|
|
| msg48173 - (view) |
Author: Michael Hudson (mwh) |
Date: 2005-06-03 15:49 |
| Logged In: YES user_id=6656 New patch, which updates marshal.c appropriately. |
|
|
| msg48174 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2007-03-05 13:36 |
| With Py3k using the long int type for all integers, do people still think this change is desirable? If so, is anybody interested in committing it? |
|
|
| msg81578 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2009-02-10 18:43 |
| I agree with MvL, it should probably be rejected. Memory size of longs is critical in py3k. |
|
|
| msg81593 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-02-10 20:00 |
| I wasn't actually proposing to reject it, merely asking, since all people who ever commented are also committers. However, since nobody bothered committing it in the last 3+ years, I'm now rejecting it. |
|
|