Array new: SmallInteger maxVal

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Array new: SmallInteger maxVal

Nicolas Cellier
Reply | Threaded
Open this post in threaded view
|

Re: Array new: SmallInteger maxVal

Eliot Miranda-2
 
Hi Nicolas, Hi Petr,

On Tue, Oct 6, 2009 at 1:05 PM, Nicolas Cellier <[hidden email]> wrote:

>From http://code.google.com/p/pharo/issues/detail?id=1282
Is this known?

It was news to me!  I'm fixing it right now in Cog.

Thanks both!
Reply | Threaded
Open this post in threaded view
|

Re: Array new: SmallInteger maxVal

David T. Lewis
In reply to this post by Nicolas Cellier
 
On Tue, Oct 06, 2009 at 10:05:02PM +0200, Nicolas Cellier wrote:
>  
> >From http://code.google.com/p/pharo/issues/detail?id=1282
> Is this known?

Thanks, it's known now :)

Also added to Mantis at http://bugs.squeak.org/view.php?id=7405.

Petr: Yes this is definitely a cool bug.

Dave

Reply | Threaded
Open this post in threaded view
|

Re: Array new: SmallInteger maxVal

David T. Lewis
In reply to this post by Nicolas Cellier
 
On Tue, Oct 06, 2009 at 10:05:02PM +0200, Nicolas Cellier wrote:
>  
> >From http://code.google.com/p/pharo/issues/detail?id=1282
> Is this known?

A performance / GC related question:

I found one part of the problem, but it is not a complete solution
(there must be more bugs).

Assume 32 bit object memory and assume that we are rigorously using
usqint rather than sqint for all OOP address calculations.

Then in Intepreter>>sufficientSpaceToInstantiate:indexableSize: if we are
trying to allocate an Array of size 16r3FFFFFFF (i.e. SmallInteger maxVal),
then we get an arithmetic overflow when we add 2500 at the end of the method.

Adding the 2500 is explained in the method comment:
   "Details: For speed, over-estimate space needed for fixed fields
    or literals; the low space threshold is a blurry line."

This addition was not present in the original Dan Ingalls version of
the method, so the overflow bug is a side effect of the optimization.

The fix would be to either a) add a check to fail the primitive if requested
size is too great, or b) remove the "add 2500" speed optimization.

Can anyone suggest which solution would be the least harmful for overall
performance? I suspect that adding the size check would be best, but I
have no data.

Dave

Reply | Threaded
Open this post in threaded view
|

Re: Array new: SmallInteger maxVal

johnmci
 

On 2009-10-09, at 7:34 AM, David T. Lewis wrote:

>
> On Tue, Oct 06, 2009 at 10:05:02PM +0200, Nicolas Cellier wrote:
>>
>>> From http://code.google.com/p/pharo/issues/detail?id=1282
>> Is this known?
>
> A performance / GC related question:
>
> I found one part of the problem, but it is not a complete solution
> (there must be more bugs).
>
> Assume 32 bit object memory and assume that we are rigorously using
> usqint rather than sqint for all OOP address calculations.
>
> Then in Intepreter>>sufficientSpaceToInstantiate:indexableSize: if  
> we are
> trying to allocate an Array of size 16r3FFFFFFF (i.e. SmallInteger  
> maxVal),
> then we get an arithmetic overflow when we add 2500 at the end of  
> the method.
>
> Adding the 2500 is explained in the method comment:
>   "Details: For speed, over-estimate space needed for fixed fields
>    or literals; the low space threshold is a blurry line."

I think that is a wild guess, in my study of the low space threshold  
issues you can
enter a state where this doesn't *fix* anything and endlessly cycle  
between allocation
and CG activity.  The solution was the bias towards growth code.

However removing the 2500 might lead to side-effects.

So much much is too big? I'd think asking for anything with the range of
0xFFFFFFFFF - 10MB to 0xFFFFFFFF  as unsigned numbers could be viewed  
as too big
given it's reasonable to consider a image would be on the order of  
10MB so you don't preclude
the ability to actually allocate somewhat close to 4GB of space.

Now obviously most  operating system won't allow that, and some can  
grind away for a long
time before deciding you are stupid as they attempt to give you the  
memory.

Still I believe the bug is not in the primitive, it does I believe  
signal primitive failure?

Lastly is this value signed? since I've seen things like Array new:  
self foobarfigureoutoptionsize.
where deep down someone in foobarfigureoutoptionsize does ^-1  to  
indicate a fatal error.



>
> This addition was not present in the original Dan Ingalls version of
> the method, so the overflow bug is a side effect of the optimization.
>
> The fix would be to either a) add a check to fail the primitive if  
> requested
> size is too great, or b) remove the "add 2500" speed optimization.
>
> Can anyone suggest which solution would be the least harmful for  
> overall
> performance? I suspect that adding the size check would be best, but I
> have no data.
>
> Dave
>

--
=
=
=
========================================================================
John M. McIntosh <[hidden email]>   Twitter:  
squeaker68882
Corporate Smalltalk Consulting Ltd.  http://www.smalltalkconsulting.com
=
=
=
========================================================================




Reply | Threaded
Open this post in threaded view
|

Re: Array new: SmallInteger maxVal

K. K. Subramaniam
In reply to this post by David T. Lewis
 
On Friday 09 October 2009 08:04:19 pm David T. Lewis wrote:
> Assume 32 bit object memory and assume that we are rigorously using
> usqint rather than sqint for all OOP address calculations.
>
> Then in Intepreter>>sufficientSpaceToInstantiate:indexableSize: if we are
> trying to allocate an Array of size 16r3FFFFFFF (i.e. SmallInteger maxVal),
> then we get an arithmetic overflow when we add 2500 at the end of the
>  method.
Practical memory allocation limit is much less than 4GB (3-3.5GB?) in 32-bit
systems because of virtual memory space reserved for process tables, file i/o
tables etc. VM will fail long before arithmetic overflow limits are hit.

Subbu
Reply | Threaded
Open this post in threaded view
|

Re: Array new: SmallInteger maxVal

David T. Lewis
In reply to this post by David T. Lewis
 
On Tue, Oct 06, 2009 at 06:58:30PM -0400, David T. Lewis wrote:

>  
> On Tue, Oct 06, 2009 at 10:05:02PM +0200, Nicolas Cellier wrote:
> >  
> > >From http://code.google.com/p/pharo/issues/detail?id=1282
> > Is this known?
>
> Thanks, it's known now :)
>
> Also added to Mantis at http://bugs.squeak.org/view.php?id=7405.
>
> Petr: Yes this is definitely a cool bug.

A fix for this is on Mantis 7405 (http://bugs.squeak.org/view.php?id=7405).

The VMMaker updates are in SqueakSource in VMMaker-dtl.142.

A separate patch is needed for platforms/unix/vm/sqUnixMemory.c (see the
Mantis report, patch also sent to Ian).

I have not tested Windows or Mac OS, but the basic scenario now is that
the VM will limit the size of memory increase requests to 31 bit integer
so support code can check for overflows. The requests from the VM are
guaranteed (I hope) be valid 31 bit integers.

Allocation requests of > 950 MB seem to be possible, although I do not
have enough memory on my PC to verify that it actually works.

Limiting allocation requests to more reasonable limits would be the
responsibility of the image.

Dave