[Patches] [ python-Patches-1177779 ] explicit sign variable for longs

[SourceForge.net]

Patches item #1177779, was opened at 2005-04-06 15:47
Message generated for change (Comment added) made by loewis
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1177779&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Michael Hudson (mwh)
Assigned to: Nobody/Anonymous (nobody)
Summary: explicit sign variable for longs

Initial Comment:
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.

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

>Comment By: Martin v. Löwis (loewis)
Date: 2007-03-05 14:36

Message:
Logged In: YES 
user_id=21627
Originator: NO

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?

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

Comment By: Michael Hudson (mwh)
Date: 2005-06-03 17:49

Message:
Logged In: YES 
user_id=6656

New patch, which updates marshal.c appropriately.

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

Comment By: Armin Rigo (arigo)
Date: 2005-04-14 11:34

Message:
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 :-)

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

Comment By: Armin Rigo (arigo)
Date: 2005-04-11 11:38

Message:
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)



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

Comment By: Michael Hudson (mwh)
Date: 2005-04-11 01:44

Message:
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"...

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

Comment By: Tim Peters (tim_one)
Date: 2005-04-11 01:28

Message:
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.


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

Comment By: Armin Rigo (arigo)
Date: 2005-04-08 16:20

Message:
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.

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

Comment By: Michael Hudson (mwh)
Date: 2005-04-06 15:51

Message:
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...

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

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