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.

classification
Title: explicit sign variable for longs
Type: Stage:
Components: Interpreter Core Versions: Python 2.5
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: arigo, loewis, mark.dickinson, mwh, pitrou, tim.peters
Priority: normal Keywords: patch

Created on 2005-04-06 13:47 by mwh, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
long-sign-less-abuse.diff mwh, 2005-04-06 13:47 mwh's patch #1
long-sign-less-abuse-2.diff mwh, 2005-04-10 23:44 mwh patch #2
long-sign-less-abuse-3.diff mwh, 2005-06-03 15:49 mwh patch #3
Messages (11)
msg48166 - (view) Author: Michael Hudson (mwh) (Python committer) 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) (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2022-04-11 14:56:10adminsetgithub: 41812
2009-02-10 20:00:24loewissetstatus: open -> closed
resolution: rejected
messages: + msg81593
2009-02-10 18:43:49pitrousetnosy: + pitrou
messages: + msg81578
2009-02-10 18:13:03ajaksu2setnosy: + mark.dickinson
2005-04-06 13:47:51mwhcreate