VM Maker: ImageFormat-dtl.49.mcz

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

VM Maker: ImageFormat-dtl.49.mcz

commits-2
 
David T. Lewis uploaded a new version of ImageFormat to project VM Maker:
http://source.squeak.org/VMMaker/ImageFormat-dtl.49.mcz

==================== Summary ====================

Name: ImageFormat-dtl.49
Author: dtl
Time: 10 January 2021, 12:39:07.044039 am
UUID: 9ae715d1-1e3e-4379-bf4a-4d45b67eb5e1
Ancestors: ImageFormat-dtl.48

Let ImageFileHeader class>>readFrom: invoke a concrete subclass for Cog/Spur or V3. Simplify instance creation.

=============== Diff against ImageFormat-dtl.48 ===============

Item was added:
+ ----- Method: CogImageFileHeader>>readFieldsFrom:littleEndian:into: (in category 'reading') -----
+ readFieldsFrom: aStream littleEndian: littleEndian into: aCollection
+ "Read data fields and answer number of bytes read"
+
+ | remainder bytesRead |
+ bytesRead := super readFieldsFrom: aStream littleEndian: littleEndian into: aCollection.
+ aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "desiredNumStackPages"
+ aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "unknownShortOrCodeSizeInKs"
+ aCollection add: (self nextNumber: 4 from: aStream littleEndian: littleEndian). "desiredEdenBytes"
+ aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "maxExtSemTabSizeSet"
+ aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "the2ndUnknownShort"
+ aCollection add: (self nextNumber: 4 from: aStream littleEndian: littleEndian). "firstSegmentBytes"
+ aCollection add: (self nextNumber: 4 from: aStream littleEndian: littleEndian). "bytesLeftInOldSpace"
+ self nextNumber: 2 from: aStream littleEndian: littleEndian.
+ remainder := headerSize - (12 * imageFormat wordSize).
+ self assert: remainder >= 0. "n.b. Mantis 7455 bug in original 64 bit image due to VMM error"
+ ^3 * imageFormat wordSize + bytesRead.
+ !

Item was removed:
- ----- Method: CogImageFileHeader>>readFieldsFrom:startingAt:headerWordSize:littleEndian:into: (in category 'reading') -----
- readFieldsFrom: aStream startingAt: imageOffset headerWordSize: headerWordSize littleEndian: littleEndian into: aCollection
- "Read data fields and answer number of bytes read"
-
- | remainder bytesRead |
- bytesRead := super readFieldsFrom: aStream startingAt: imageOffset headerWordSize: headerWordSize littleEndian: littleEndian into: aCollection.
- aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "desiredNumStackPages"
- aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "unknownShortOrCodeSizeInKs"
- aCollection add: (self nextNumber: 4 from: aStream littleEndian: littleEndian). "desiredEdenBytes"
- aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "maxExtSemTabSizeSet"
- aCollection add: (self nextNumber: 2 from: aStream littleEndian: littleEndian). "the2ndUnknownShort"
- aCollection add: (self nextNumber: 4 from: aStream littleEndian: littleEndian). "firstSegmentBytes"
- aCollection add: (self nextNumber: 4 from: aStream littleEndian: littleEndian). "bytesLeftInOldSpace"
- self nextNumber: 2 from: aStream littleEndian: littleEndian.
- remainder := headerSize - (12 * imageFormat wordSize).
- self assert: remainder >= 0. "n.b. Mantis 7455 bug in original 64 bit image due to VMM error"
- ^3 * imageFormat wordSize + bytesRead.
- !

Item was added:
+ ----- Method: ImageFileHeader class>>concreteClass: (in category 'instance creation') -----
+ concreteClass: imageFormat
+
+ imageFormat requiresNativeFloatWordOrder
+ ifTrue: [ ^CogImageFileHeader ]
+ ifFalse: [ ^self ]!

Item was changed:
  ----- Method: ImageFileHeader class>>readFrom:startingAt: (in category 'instance creation') -----
  readFrom: aStream startingAt: imageOffset
 
+ ^self readFrom: aStream startingAt: imageOffset into: OrderedCollection new!
- ^self basicNew readFrom: aStream startingAt: imageOffset into: OrderedCollection new!

Item was added:
+ ----- Method: ImageFileHeader class>>readFrom:startingAt:into: (in category 'private') -----
+ readFrom: aStream startingAt: imageOffset into: aCollection
+
+ | formatInfo imageFormat cls hdr remainder bytesRead |
+ formatInfo := self readImageVersionFrom: aStream startingAt: imageOffset.
+ imageFormat := formatInfo first.
+ aCollection add: imageFormat asInteger.
+ cls := self concreteClass: imageFormat.
+ hdr := cls new imageFormat: imageFormat.
+ bytesRead := hdr readFieldsFrom: aStream littleEndian: formatInfo second into: aCollection.
+ remainder := hdr headerSize - bytesRead.
+ self assert: remainder >= 0. "n.b. Mantis 7455 bug in original 64 bit image due to VMM error"
+ aStream next: (hdr headerSize - bytesRead).
+ ^hdr fromEntryStream: aCollection readStream.
+ !

Item was added:
+ ----- Method: ImageFileHeader class>>readImageVersionFrom:startingAt: (in category 'private') -----
+ readImageVersionFrom: aStream startingAt: imageOffset
+ "Look for image format in the next 4 or 8 bytes and set imageFormat. Answer a
+ two element array with image format, and a boolean true if the header is written
+ in little endian format."
+
+ | imageFormat littleEndian |
+ (aStream nextNumber: 4) caseOf:
+ {
+ [ 16r00001966 "6502" ] -> [ imageFormat := ImageFormat fromInteger: 6502. littleEndian := false ] .
+ [ 16r66190000 "6502" ] -> [ imageFormat := ImageFormat fromInteger: 6502. littleEndian := true  ] .
+ [ 16r00001968 "6504" ] -> [ imageFormat := ImageFormat fromInteger: 6504. littleEndian := false ] .
+ [ 16r68190000 "6504" ] -> [ imageFormat := ImageFormat fromInteger: 6504. littleEndian := true ] .
+ [ 16r00001969 "6505" ] -> [ imageFormat := ImageFormat fromInteger: 6505. littleEndian := false ] .
+ [ 16r69190000 "6505" ] -> [ imageFormat := ImageFormat fromInteger: 6505. littleEndian := true ] .
+ [ 16r00001979 "6521" ] -> [ imageFormat := ImageFormat fromInteger: 6521. littleEndian := false ] .
+ [ 16r79190000 "6521" ] -> [ imageFormat := ImageFormat fromInteger: 6521. littleEndian := true ] .
+ [ 16rA0090100 "68000" ] -> [ imageFormat := ImageFormat fromInteger: 68000. aStream next: 4. littleEndian := true ] . "format number in 64 bit integer"
+ [ 16rA2090100 "68002" ] -> [ imageFormat := ImageFormat fromInteger: 68002. aStream next: 4. littleEndian := true ] .
+ [ 16rA3090100 "68003" ] -> [ imageFormat := ImageFormat fromInteger: 68003. aStream next: 4. littleEndian := true ] .
+ [ 16rB3090100 "68019" ] -> [ imageFormat := ImageFormat fromInteger: 68019. littleEndian := true ] . "Cog/Spur always puts format number in first 32 bits"
+ [ 16r000109B3 "68019" ] -> [ imageFormat := ImageFormat fromInteger: 68019. littleEndian := false ] .
+ [ 16rB5090100 "68021" ] -> [ imageFormat := ImageFormat fromInteger: 68021. littleEndian := true ] .
+ [ 16r000109B5 "68021" ] -> [ imageFormat := ImageFormat fromInteger: 68021. littleEndian := false ] .
+ [ 16r00000000 ] -> [
+ "Standard interpreter VM puts the format number in the first 64 bits for a 64 bit image, so
+ the leading 4 bytes are zero in this case. Cog/Spur VMs put the format number in the first
+ 32 bits for both 32 and 64 bit images."
+ (aStream nextNumber: 4) caseOf: {
+ [ 16r000109A0 "68000" ] -> [ imageFormat := ImageFormat fromInteger: 68000. littleEndian := false ] .
+ [ 16r000109A2 "68002" ] -> [ imageFormat := ImageFormat fromInteger: 68002. littleEndian := false ] .
+ [ 16r000109A3 "68003" ] -> [ imageFormat := ImageFormat fromInteger: 68003. littleEndian := false ]
+ } otherwise: [self error: self asString , ' unrecognized format number']
+ ]
+ } otherwise: [self error: self asString , ' unrecognized format number'].
+ ^ { imageFormat . littleEndian }
+
+ "ImageFormat versionNumberByteArrays do: [:e |
+ Transcript cr; show: e printString , ': ', (ImageFormat fromBytes: e) description]
+
+ #[0 0 25 102]: a 32-bit image with no closure support and no native platform float word order requirement (6502)
+ #[102 25 0 0]: a 32-bit image with no closure support and no native platform float word order requirement (6502)
+ #[0 0 25 104]: a 32-bit image with closure support and no native platform float word order requirement (6504)
+ #[104 25 0 0]: a 32-bit image with closure support and no native platform float word order requirement (6504)
+ #[0 0 0 0 0 1 9 160]: a 64-bit image with no closure support and no native platform float word order requirement (68000)
+ #[160 9 1 0 0 0 0 0]: a 64-bit image with no closure support and no native platform float word order requirement (68000)
+ #[0 0 0 0 0 1 9 162]: a 64-bit image with closure support and no native platform float word order requirement (68002)
+ #[162 9 1 0 0 0 0 0]: a 64-bit image with closure support and no native platform float word order requirement (68002)
+ #[0 0 25 105]: a 32-bit image with closure support and float words stored in native platform order (6505)
+ #[105 25 0 0]: a 32-bit image with closure support and float words stored in native platform order (6505)
+ #[0 0 0 0 0 1 9 163]: a 64-bit image with closure support and float words stored in native platform order (68003)
+ #[163 9 1 0 0 0 0 0]: a 64-bit image with closure support and float words stored in native platform order (68003)
+ #[0 0 25 121]: a 32-bit image with closure support and float words stored in native platform order using Spur object format (6521)
+ #[121 25 0 0]: a 32-bit image with closure support and float words stored in native platform order using Spur object format (6521)
+ #[0 0 0 0 0 1 9 179]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (obsolete) (68019)
+ #[179 9 1 0 0 0 0 0]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (obsolete) (68019)
+ #[0 0 0 0 0 1 9 181]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (68021)
+ #[181 9 1 0 0 0 0 0]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (68021)
+ "
+ !

Item was changed:
+ ----- Method: ImageFileHeader>>packedLongSize (in category 'converting') -----
- ----- Method: ImageFileHeader>>packedLongSize (in category 'writing') -----
  packedLongSize
  "Standard interpreter VM writes integers to the file header as 64 bits for
  a 64 bit image or 32 bits for a 32 bit image. Cog/Spur VMs use 32 bits
  where possible, otherwise 64 bits for long values on a 64 bit system."
 
  ^ imageFormat wordSize!

Item was added:
+ ----- Method: ImageFileHeader>>readFieldsFrom:littleEndian:into: (in category 'reading') -----
+ readFieldsFrom: aStream littleEndian: littleEndian into: aCollection
+ "Read data fields and answer number of bytes read"
+
+ | remainder screenSizeWord |
+ headerSize := self nextNumber: self packedLongSize from: aStream littleEndian: littleEndian.
+ aCollection add: headerSize.
+ aCollection add: ( self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "imageBytes"
+ aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "startOfMemory"
+ aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "specialObjectsOop"
+ aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "lastHash"
+ screenSizeWord := self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian.
+ aCollection add: ((screenSizeWord >> 16) @ (screenSizeWord bitAnd: 16rFFFF)).
+ aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "imageHeaderFlags"
+ aCollection add: (self nextNumber: self packedLongSize from: aStream littleEndian: littleEndian). "extraVMMemory"
+ remainder := headerSize - (9 * imageFormat wordSize).
+ self assert: remainder >= 0. "n.b. Mantis 7455 bug in original 64 bit image due to VMM error"
+ ^9 * imageFormat wordSize.
+ !

Item was removed:
- ----- Method: ImageFileHeader>>readFieldsFrom:startingAt:headerWordSize:littleEndian:into: (in category 'reading') -----
- readFieldsFrom: aStream startingAt: imageOffset headerWordSize: headerWordSize littleEndian: littleEndian into: aCollection
- "Read data fields and answer number of bytes read"
-
- | remainder screenSizeWord |
- headerSize := self nextNumber: headerWordSize from: aStream littleEndian: littleEndian.
- aCollection add: headerSize.
- aCollection add: ( self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "imageBytes"
- aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "startOfMemory"
- aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "specialObjectsOop"
- aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "lastHash"
- screenSizeWord := self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian.
- aCollection add: ((screenSizeWord >> 16) @ (screenSizeWord bitAnd: 16rFFFF)).
- aCollection add: (self nextNumber: imageFormat wordSize from: aStream littleEndian: littleEndian). "imageHeaderFlags"
- aCollection add: (self nextNumber: headerWordSize from: aStream littleEndian: littleEndian). "extraVMMemory"
- remainder := headerSize - (9 * imageFormat wordSize).
- self assert: remainder >= 0. "n.b. Mantis 7455 bug in original 64 bit image due to VMM error"
- ^9 * imageFormat wordSize.
- !

Item was removed:
- ----- Method: ImageFileHeader>>readFrom:startingAt:into: (in category 'reading') -----
- readFrom: aStream startingAt: imageOffset into: aCollection
-
- | remainder bytesRead headerWordSize littleEndian |
- littleEndian := self readImageVersionFrom: aStream startingAt: imageOffset.
- headerWordSize := aStream position - imageOffset.
- aCollection add: imageFormat asInteger.
- bytesRead := self readFieldsFrom: aStream startingAt: imageOffset headerWordSize: headerWordSize littleEndian: littleEndian into: aCollection.
- remainder := headerSize - bytesRead.
- self assert: remainder >= 0. "n.b. Mantis 7455 bug in original 64 bit image due to VMM error"
- aStream next: (headerSize - bytesRead).
-
- self fromEntryStream: aCollection readStream.
- !

Item was removed:
- ----- Method: ImageFileHeader>>readImageVersionFrom:startingAt: (in category 'reading') -----
- readImageVersionFrom: aStream startingAt: imageOffset
- "Look for image format in the next 4 or 8 bytes and set imageFormat. Answer true
- if the header is written in little endian format."
-
- (aStream nextNumber: 4) caseOf:
- {
- [ 16r00001966 "6502" ] -> [ imageFormat := ImageFormat fromInteger: 6502. ^false ] .
- [ 16r66190000 "6502" ] -> [ imageFormat := ImageFormat fromInteger: 6502. ^true ] .
- [ 16r00001968 "6504" ] -> [ imageFormat := ImageFormat fromInteger: 6504. ^false ] .
- [ 16r68190000 "6504" ] -> [ imageFormat := ImageFormat fromInteger: 6504. ^true ] .
- [ 16r00001969 "6505" ] -> [ imageFormat := ImageFormat fromInteger: 6505. ^false ] .
- [ 16r69190000 "6505" ] -> [ imageFormat := ImageFormat fromInteger: 6505. ^true ] .
- [ 16r00001979 "6521" ] -> [ imageFormat := ImageFormat fromInteger: 6521. ^false ] .
- [ 16r79190000 "6521" ] -> [ imageFormat := ImageFormat fromInteger: 6521. ^true ] .
- [ 16rA0090100 "68000" ] -> [ imageFormat := ImageFormat fromInteger: 68000. aStream next: 4. ^true ] . "format number in 64 bit integer"
- [ 16rA2090100 "68002" ] -> [ imageFormat := ImageFormat fromInteger: 68002. aStream next: 4. ^true ] .
- [ 16rA3090100 "68003" ] -> [ imageFormat := ImageFormat fromInteger: 68003. aStream next: 4. ^true ] .
- [ 16rB3090100 "68019" ] -> [ imageFormat := ImageFormat fromInteger: 68019. ^true ] . "Cog/Spur always puts format number in first 32 bits"
- [ 16r000109B3 "68019" ] -> [ imageFormat := ImageFormat fromInteger: 68019. ^false ] .
- [ 16rB5090100 "68021" ] -> [ imageFormat := ImageFormat fromInteger: 68021. ^true ] .
- [ 16r000109B5 "68021" ] -> [ imageFormat := ImageFormat fromInteger: 68021. ^false ] .
- [ 16r00000000 ] -> [
- "Standard interpreter VM puts the format number in the first 64 bits for a 64 bit image, so
- the leading 4 bytes are zero in this case. Cog/Spur VMs put the format number in the first
- 32 bits for both 32 and 64 bit images."
- (aStream nextNumber: 4) caseOf: {
- [ 16r000109A0 "68000" ] -> [ imageFormat := ImageFormat fromInteger: 68000. ^false ] .
- [ 16r000109A2 "68002" ] -> [ imageFormat := ImageFormat fromInteger: 68002. ^false ] .
- [ 16r000109A3 "68003" ] -> [ imageFormat := ImageFormat fromInteger: 68003. ^false ]
- } otherwise: [self error: self asString , ' unrecognized format number']
- ]
- } otherwise: [self error: self asString , ' unrecognized format number']
-
- "ImageFormat versionNumberByteArrays do: [:e |
- Transcript cr; show: e printString , ': ', (ImageFormat fromBytes: e) description]
-
- #[0 0 25 102]: a 32-bit image with no closure support and no native platform float word order requirement (6502)
- #[102 25 0 0]: a 32-bit image with no closure support and no native platform float word order requirement (6502)
- #[0 0 25 104]: a 32-bit image with closure support and no native platform float word order requirement (6504)
- #[104 25 0 0]: a 32-bit image with closure support and no native platform float word order requirement (6504)
- #[0 0 0 0 0 1 9 160]: a 64-bit image with no closure support and no native platform float word order requirement (68000)
- #[160 9 1 0 0 0 0 0]: a 64-bit image with no closure support and no native platform float word order requirement (68000)
- #[0 0 0 0 0 1 9 162]: a 64-bit image with closure support and no native platform float word order requirement (68002)
- #[162 9 1 0 0 0 0 0]: a 64-bit image with closure support and no native platform float word order requirement (68002)
- #[0 0 25 105]: a 32-bit image with closure support and float words stored in native platform order (6505)
- #[105 25 0 0]: a 32-bit image with closure support and float words stored in native platform order (6505)
- #[0 0 0 0 0 1 9 163]: a 64-bit image with closure support and float words stored in native platform order (68003)
- #[163 9 1 0 0 0 0 0]: a 64-bit image with closure support and float words stored in native platform order (68003)
- #[0 0 25 121]: a 32-bit image with closure support and float words stored in native platform order using Spur object format (6521)
- #[121 25 0 0]: a 32-bit image with closure support and float words stored in native platform order using Spur object format (6521)
- #[0 0 0 0 0 1 9 179]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (obsolete) (68019)
- #[179 9 1 0 0 0 0 0]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (obsolete) (68019)
- #[0 0 0 0 0 1 9 181]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (68021)
- #[181 9 1 0 0 0 0 0]: a 64-bit image with closure support and float words stored in native platform order using Spur object format (68021)
- "
- !

Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: ImageFormat-dtl.49.mcz

K K Subbu
 
On 10/01/21 5:39 am, [hidden email] wrote:
> + ----- Method: CogImageFileHeader>>readFieldsFrom:littleEndian:into: (in category 'reading') -----
> + readFieldsFrom: aStream littleEndian: littleEndian into: aCollection

Passing endian flag to every read looks like an overkill, doesn't it?

> + [ 16r00001966 "6502" ] -> [ imageFormat := ImageFormat fromInteger: 6502. littleEndian := false ] .

The endianness is fixed when the imageFormat is instantiated. If it
could be encapsulated as part of this instance, then there is no need to
pass it as an argument for every read from the store associated with
this image instance.

Ideally, the image store type (little/big endian) should be separated
from the image encoding format (32-bit, 64-bit, ....). But I guess this
separation can evolve over time.

Regards .. Subbu
Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: ImageFormat-dtl.49.mcz

David T. Lewis
 
On Sun, Jan 10, 2021 at 11:30:50AM +0530, K K Subbu wrote:

>
> On 10/01/21 5:39 am, [hidden email] wrote:
> >+ ----- Method: CogImageFileHeader>>readFieldsFrom:littleEndian:into: (in
> >category 'reading') -----
> >+ readFieldsFrom: aStream littleEndian: littleEndian into: aCollection
>
> Passing endian flag to every read looks like an overkill, doesn't it?
>
> >+ [ 16r00001966 "6502" ] -> [ imageFormat :=
> >ImageFormat fromInteger: 6502. littleEndian := false ] .
>
> The endianness is fixed when the imageFormat is instantiated. If it
> could be encapsulated as part of this instance, then there is no need to
> pass it as an argument for every read from the store associated with
> this image instance.
>
> Ideally, the image store type (little/big endian) should be separated
> from the image encoding format (32-bit, 64-bit, ....). But I guess this
> separation can evolve over time.
>
> Regards .. Subbu

Hi Subbu,

It is an interesting question and I'm glad you spotted it. In the VM,
the logic for reading and writing the snapshot header is buried in the
code where most people will never see or think about it. So I am trying
to make it more accessible.

Although I have reorganized it a bit, the code that you are quoting
is patterned after the actual image loading methods in VMMaker. When
loading an image file, the very first thing that happens is that the
first few bytes of the file are read and decoded. The method that does
this figures out the endianness as a side effect of decoding the image
format number, so it is a function with two side effects. At this point
it knows the endianness of the data stored in the image file, which
will be the endianness of whatever machine actually saved the file,
not the endianness of the machine that is currently loading the file.

This endianness information is needed only for the remainder of the
image loading process. Once the image is loaded into memory, and
before the VM begins actually running that image, the endianness
flag is no longer required and can be discarded.

The reason for this complication is that we want an image file that
was saved by a big endian machine to be loadable on a little endian
machine and vice versa. We also want this to be fast and efficient,
so the policy is to always write an image file in the natural endianness
of the machine that is running it, and do extra byte swapping if and
only if the image file is being loaded by a different kind of computer.

This was a design decision that was made many years ago, and it
still works well today. But computers are a lot faster today, and
it might be perfectly legitimate to adopt a policy of always saving
an image file in e.g. network byte order. With today's fast computers,
it is possible that you would never notice any performance impact.
I have never tried it, but this might be an interesting experiment
for a student to try.

Dave
Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: ImageFormat-dtl.49.mcz

David T. Lewis
 
On Sun, Jan 10, 2021 at 11:11:30AM -0500, David T. Lewis wrote:

>  
> On Sun, Jan 10, 2021 at 11:30:50AM +0530, K K Subbu wrote:
> >
> > On 10/01/21 5:39 am, [hidden email] wrote:
> > >+ ----- Method: CogImageFileHeader>>readFieldsFrom:littleEndian:into: (in
> > >category 'reading') -----
> > >+ readFieldsFrom: aStream littleEndian: littleEndian into: aCollection
> >
> > Passing endian flag to every read looks like an overkill, doesn't it?
> >
> > >+ [ 16r00001966 "6502" ] -> [ imageFormat :=
> > >ImageFormat fromInteger: 6502. littleEndian := false ] .
> >
> > The endianness is fixed when the imageFormat is instantiated. If it
> > could be encapsulated as part of this instance, then there is no need to
> > pass it as an argument for every read from the store associated with
> > this image instance.
> >
> > Ideally, the image store type (little/big endian) should be separated
> > from the image encoding format (32-bit, 64-bit, ....). But I guess this
> > separation can evolve over time.
> >
> > Regards .. Subbu
>
> Hi Subbu,
>
> It is an interesting question and I'm glad you spotted it. In the VM,
> the logic for reading and writing the snapshot header is buried in the
> code where most people will never see or think about it. So I am trying
> to make it more accessible.
>

I should add one more thing - I did not really answer your question of
whether the image file endianness should be saved as an instance variable
somewhere, as opposed to passing it around as a flag. I don't really have
a good answer right now. For the case of simply reading and writing an
ImageFileHeader, I think it's better to leave at as a flag used only in
the process of reading. But I can imagine other use cases where it might
be useful to keep it as an instance variable.

Dave
Reply | Threaded
Open this post in threaded view
|

Re: VM Maker: ImageFormat-dtl.49.mcz

K K Subbu
In reply to this post by David T. Lewis
 
On 10/01/21 9:41 pm, David T. Lewis wrote:
> Although I have reorganized it a bit, the code that you are quoting
> is patterned after the actual image loading methods in VMMaker. When
> loading an image file, the very first thing that happens is that the
> first few bytes of the file are read and decoded. The method that does
> this figures out the endianness as a side effect of decoding the image
> format number, so it is a function with two side effects. At this point
> it knows the endianness of the data stored in the image file, which
> will be the endianness of whatever machine actually saved the file,
> not the endianness of the machine that is currently loading the file.

This dumping of object graph into image file worked well as long there
were only one or two formats. But now there are so many image variants
that stream reader/writers like in GIF or PNG would be simpler and
cleaner (ImageReader/ImageWriter?). It would also allow new VMs to
"import" all old formats.

You're right about byte-swap cost not being significant on modern
machines. If htonx/ntohx works for network i/o it should work better for
storage I/O. Perhaps, the next iteration of image format could settle on
network byte order and eliminate endian image variants.

> This endianness information is needed only for the remainder of the
> image loading process. Once the image is loaded into memory, and
> before the VM begins actually running that image, the endianness
> flag is no longer required and can be discarded.

The VM works with only one image at a time, so the cost of one property
in ImageReader singleton will be small compared to simplification of
messages. The property will also be reclaimed when the stream is closed.

Regards .. Subbu