Why spaceOkay test needed?

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

Why spaceOkay test needed?

Adrian Lienhard
 
Out of curiosity, does anybody know why in...

primitiveNew
        "Allocate a new fixed-size instance. Fail if the allocation would  
leave less than lowSpaceThreshold bytes free. May cause a GC"
        | class spaceOkay |
        class := self stackTop.
        "The following may cause GC!"
        spaceOkay := self sufficientSpaceToInstantiate: class indexableSize: 0.
        self success: spaceOkay.
        successFlag ifTrue: [ self push: (self instantiateClass: self  
popStack indexableSize: 0) ]

... there is the call to #sufficientSpaceToInstantiate:indexableSize:  
although #instantiateClass:indexableSize: also indirectly calls  
#sufficientSpaceToAllocate:?
Both, the primitive fallback code and the vm code signal the low  
space sem in case of failure.

Adrian
Reply | Threaded
Open this post in threaded view
|

Re: Why spaceOkay test needed?

Andreas.Raab
 
Adrian Lienhard wrote:

>
> Out of curiosity, does anybody know why in...
>
> primitiveNew
>     "Allocate a new fixed-size instance. Fail if the allocation would
> leave less than lowSpaceThreshold bytes free. May cause a GC"
>     | class spaceOkay |
>     class := self stackTop.
>     "The following may cause GC!"
>     spaceOkay := self sufficientSpaceToInstantiate: class indexableSize: 0.
>     self success: spaceOkay.
>     successFlag ifTrue: [ self push: (self instantiateClass: self
> popStack indexableSize: 0) ]
>
> ... there is the call to #sufficientSpaceToInstantiate:indexableSize:
> although #instantiateClass:indexableSize: also indirectly calls
> #sufficientSpaceToAllocate:?

Where does it do that?

> Both, the primitive fallback code and the vm code signal the low space
> sem in case of failure.

The fallback code in the image is broken IMO. It needs to signal
OutOfMemory instead of the low space semaphore. We had our servers go
belly-up because of malformed requests that requested a 1 gigabyte or
more memory.

Cheers,
   - Andreas
Reply | Threaded
Open this post in threaded view
|

Re: Why spaceOkay test needed?

Adrian Lienhard
 

On Aug 20, 2007, at 18:37 , Andreas Raab wrote:

> Adrian Lienhard wrote:
>> Out of curiosity, does anybody know why in...
>> primitiveNew
>>     "Allocate a new fixed-size instance. Fail if the allocation  
>> would leave less than lowSpaceThreshold bytes free. May cause a GC"
>>     | class spaceOkay |
>>     class := self stackTop.
>>     "The following may cause GC!"
>>     spaceOkay := self sufficientSpaceToInstantiate: class  
>> indexableSize: 0.
>>     self success: spaceOkay.
>>     successFlag ifTrue: [ self push: (self instantiateClass: self  
>> popStack indexableSize: 0) ]
>> ... there is the call to  
>> #sufficientSpaceToInstantiate:indexableSize: although  
>> #instantiateClass:indexableSize: also indirectly calls  
>> #sufficientSpaceToAllocate:?
>
> Where does it do that?

instantiateClass:indexableSize: ->  
allocate:headerSize:h1:h2:h3:doFill:with: -> allocateChunk: ->  
sufficientSpaceToAllocate:

Adrian
Reply | Threaded
Open this post in threaded view
|

Re: Why spaceOkay test needed?

Mathieu SUEN
In reply to this post by Andreas.Raab
 
So #sufficientSpaceToInstantiate:indexableSize: call can be removed?

        Mth



On Aug 20, 2007, at 6:37 PM, Andreas Raab wrote:

> Adrian Lienhard wrote:
>> Out of curiosity, does anybody know why in...
>> primitiveNew
>>     "Allocate a new fixed-size instance. Fail if the allocation  
>> would leave less than lowSpaceThreshold bytes free. May cause a GC"
>>     | class spaceOkay |
>>     class := self stackTop.
>>     "The following may cause GC!"
>>     spaceOkay := self sufficientSpaceToInstantiate: class  
>> indexableSize: 0.
>>     self success: spaceOkay.
>>     successFlag ifTrue: [ self push: (self instantiateClass: self  
>> popStack indexableSize: 0) ]
>> ... there is the call to  
>> #sufficientSpaceToInstantiate:indexableSize: although  
>> #instantiateClass:indexableSize: also indirectly calls  
>> #sufficientSpaceToAllocate:?
>
> Where does it do that?
>
>> Both, the primitive fallback code and the vm code signal the low  
>> space sem in case of failure.
>
> The fallback code in the image is broken IMO. It needs to signal  
> OutOfMemory instead of the low space semaphore. We had our servers  
> go belly-up because of malformed requests that requested a 1  
> gigabyte or more memory.
>
> Cheers,
>   - Andreas

Reply | Threaded
Open this post in threaded view
|

Re: Why spaceOkay test needed?

timrowledge
 

On 23-Aug-07, at 3:52 PM, Mathieu Suen wrote:

> So #sufficientSpaceToInstantiate:indexableSize: call can be removed?

 From where?

You might *possibly* be able to make a good argument for removing it  
from within the instantiateClass:indexableSize: depending on its  
usage. I wouldn't recommend removing it from the primitiveNew code  
though; how else are you going to  be able to fail the prim cleanly?  
Sure, you could make the instantiateClass:indexableSize: code return  
nil if the instantiation cannot be performed. Then you have to modify  
all senders to handle that eventuality. You'd have to not push: the  
nil though.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
A conscience is what hurts when all your other parts feel so good.