[Vm-dev] [Fwd: Image file loading]

[Andreas Raab]

Igor Stasenko wrote:
> 2010/1/20 Andreas Raab <andreas.raab at gmx.de>:
>> The platform code reads the allocates memory and reads the image. It can do
>> this by reading a file or (in the Android case) by doing a series of JNI
>> calls that pass uncompressed portions of a chunked image file from the app
>> package (yes, it is complicated on Android). Or the support code could read
>> it from a socket connection. Whatever. There is no reason for the
>> interpreter to be involved *at all* in reading the image file.
>>
> Well, the current code implies some logic when decoding a header, like
> allocating enough space + breathing room for object memory.

That's a problem the support code already deals with. The support code 
already knows the limits on memory that the user specified (via -memory 
argument or implicitly via mmap() or similar means) and it also already 
needs to check that the image can actually fit into the heap (or else 
it'll blow up when trying to read it). So it's similarly trivial for the 
support code to say:

   if(heapSize < fsize(imageFile) + headRoom)
     abort("Not enough memory to load image");

> So, i thought that its better when interpreter controls the memory
> allocation size, not platform code alone.

The platform already completely controls the allocation. It's only that 
we allow the interpreter to make the first request and that request is 
based on what we've passed in as desiredHeapSize from the support code. 
Lots of useless complexity, the support code might as well just allocate 
it without involving the interpreter.

Cheers,
   - Andreas

> 
>> Cheers,
>>  - Andreas
>>
>>> No file access, no squeakFileOffsetType mess. Nothing.
>>> A platform code deciding these details by own!
>>>
>>> 2010/1/20 Andreas Raab <andreas.raab at gmx.de>:
>>>> David T. Lewis wrote:
>>>>> On Tue, Jan 19, 2010 at 02:30:43PM -0800, Andreas Raab wrote:
>>>>>> What I'm proposing is an additional VM entry point, called
>>>>>>
>>>>>>  Interpreter>>readImageFromHeap: heapStart size: heapSize
>>>>>>
>>>>>> which takes to arguments: The pointer to the beginning of the
>>>>>> (pre-allocated) memory portion where the image file has been loaded or
>>>>>> mapped and a size for the allocated memory portion. The VM then assumes
>>>>>> that beginning at heapStart there is an image provided that it needs to
>>>>>> properly prepare (i.e., perform byte-reversal, pointer adjustment etc).
>>>>> A suggestion:
>>>>>
>>>>>  Interpreter>>installImage: address size: imageSize header: header
>>>> I don't like that. It implies that the support code needs to be able to
>>>> dissect the image file to find out how much is header and how much is
>>>> data.
>>>> That implies the support code needs to know about image format, byte
>>>> reversal, and several other things. The interpreter already does that so
>>>> why
>>>> should we have to duplicate that in the support code again?
>>>>
>>>>> Rationale:
>>>>>
>>>>> - "Install" is more appropriate that "read", since the contents of the
>>>>>  image file have already been read.
>>>>>
>>>>> - "Heap" is not relevant.
>>>> Fair enough. I don't really care what it's called.
>>>>
>>>>> - The file header is logically distinct from from the contents of the
>>>>>  image proper, and since someone else did the reading, it seems better
>>>>>  to pass them as two distinct things (*). The pointer to header is
>>>>>  presumed to be a memory block of at least 64 bytes, so in the usual
>>>>>  case of a 32 bit image read into a block of malloc'ed space, we expect
>>>>>  that address == header + 64.
>>>> See above. You're making it artificially harder for the support code by
>>>> requiring it to dissect the file and figure out what the header is. There
>>>> is
>>>> really no point to that.
>>>>
>>>>> My opinion: Adding the entry point is easy to do and has a clear benefit
>>>>> for at least one platform of interest, so we should do this. Removing
>>>>> the existing entry point is debatable and can be done at a later time.
>>>> Why do you think removing the existing entry point is debatable? I can't
>>>> see
>>>> a long-term benefit for using readImageFromFile: given that the new entry
>>>> point trivially subsumes the old one.
>>>>
>>>> Cheers,
>>>>  - Andreas
>>>>
>>>
>>>
> 
> 
>