[Vm-dev] the initial memory allocation interface...

[Eliot Miranda]

...stinks.
I'm talking about sqAllocateMemory.  Here's the base definition from
platforms/Cross/vm/sq.h:

#define sqAllocateMemory(minHeapSize, desiredHeapSize)
 malloc(desiredHeapSize)

The problem here is that there's no obvious way for the client to know how
much memory is returned.  The signature implies it is somewhere
between minHeapSize & desiredHeapSize inclusive (or null, if allocation
failed).  But how do you tell?  Well, one could always ask to grow memory by
0 and see what you get back, except for the small chicken-and-egg problem
that sqGrowMemory:By: and sqShrinkMemory:By: require the ammount of memory
allocated as an argument:

#define sqGrowMemoryBy(oldLimit, delta)         oldLimit
#define sqShrinkMemoryBy(oldLimit, delta)       oldLimit

So one has to go to extraordinary lengths that are completely non-obvious in
the client code to actually pass-back the ammount allocated.  Here's a
client in readImageFromFile:

        "allocate a contiguous block of memory for the Squeak heap"
        memory *:=* self cCode: 'sqAllocateMemory(minimumMemory, heapSize)'
.
        memory = nil ifTrue: [self insufficientMemoryAvailableError].

        memStart *:=* self startOfMemory.
        self setMemoryLimit: (memStart + heapSize) - 24. "decrease
memoryLimit a tad for safety"
        self setEndOfMemory: memStart + dataSize.

and here's how one gets around the absence of a "how much was allocated"
parameter, from platforms/Mac OS/vm/sqPlatformSpecifc.h

#undef sqAllocateMemory

usqInt      sqAllocateMemoryMac(sqInt minHeapSize, sqInt *desiredHeapSize);

#define sqAllocateMemory(x,y) sqAllocateMemoryMac(x,&y);

Ingenious (seriously; John this is a very clever working around of the
restriction).  Totally non-obvious at the client site, but it still gets the
job done without requiring the client to change.  So this costs too much to
maintain (a.k.a. puts too much strain on my rodent brain).

We could go with an explicit out parameter, as in

        "allocate a contiguous block of memory for the Squeak heap"
        memory *:=* self cCode: 'sqAllocateMemory(minimumMemory, heapSize,
&allocatedSize)'.
        memory = nil ifTrue: [self insufficientMemoryAvailableError].

        memStart *:=* self startOfMemory.
        self setMemoryLimit: (memStart + allocatedSize) - 24. "decrease
memoryLimit a tad for safety"
        self setEndOfMemory: memStart + dataSize.

but out parameters are not supported by Smalltalk and the VM is a Smalltalk
program goddammit.

So I want to canvas opinions on the following simplified proposal, which is
to drop the oldLimit parameter from sqGrowMemoryBy, discard sqShrinkMemoryBy
(it is superfluous given sqGrowMemoryBy can take a signed argument) and use
self sqGrowMemoryBy: 0 to determine how much memory has been allocated:

        "allocate a contiguous block of memory for the Squeak heap"
        memory *:=* self sqAllocateAtLeast: minimumMemory AtMostMemory:
heapSize.
        memory = nil ifTrue: [self insufficientMemoryAvailableError].
        heapSize *:=* self sqGrowMemoryBy: 0.
        memStart *:=* self startOfMemory.
        self setMemoryLimit: (memStart + heapSize) - 24. "decrease
memoryLimit a tad for safety"
        self setEndOfMemory: memStart + dataSize.

with the obvious default implementations in e.g.
platforms/Cross/vm/sqMemory.c

static unsigned long memoryLimit = 0;

void *
sqAllocateAtLeastAtMostMemory(unsigned long committed, unsigned long
reserved)
{
   void *mem;
    if ((mem = malloc(reserved)) ~= 0)
        memoryLimit = reserved;
    else if ((mem = malloc(committed)) ~= 0)
        memoryLimit = committed;
    return mem;
}

unsigned long
sqGrowMemoryBy(sqInt delta)
{
    return memoryLimit;
}

and everywhere in the VM which expects the old sqGrowMemoryBy to answer a
pointer to the limit must instead add the result to the base of allocated
memory; i.e.

        limit *:=* self sqGrowMemory: memoryLimit By: delta.
        limit = memoryLimit ifFalse:
                 [self setMemoryLimit: limit - 24. "remove a tad for safety"

                  self initializeMemoryFirstFree: freeBlock]

is rewritten as

        limit *:=* memoryLimit + (self sqGrowMemoryBy: delta).
        limit = memoryLimit ifFalse:
                 [self setMemoryLimit: limit - 24. "remove a tad for safety"

                  self initializeMemoryFirstFree: freeBlock]
Opinions?

best
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20090724/4f7206b2/attachment.htm