theseion / Fuel

Fuel, the Smalltalk object serializer
https://theseion.github.io/Fuel
MIT License
27 stars 13 forks source link

32-bit Serialized Blocks Can't Be Materialized in 64-bits #234

Closed seandenigris closed 3 years ago

seandenigris commented 6 years ago

I reported the following problem on Pharo-Dev:

  1. Serialized a block in #60540 32-bit
  2. Materialized it in same version 64-bit and it was "broken", i.e.: a. It can't be serialized again as indicated above b. Strangely, its print string in the GT Inspector is the entire source code of #outerContext instead of the source code of just the block

aBlock:

Eliot Miranda answered:

Fuel is not adjusting the pcs in Contexts and BlockClosures when loading something saved in a different word size. CompiledCode (CompiledMethod & CompiledBlock) is a hybrid object, the first part being object references (the method’s header followed by its literals), the second part being bytes (the bytecodes for the method and any additional info encoded in trailing bytes). So in 32 bits a pc is (4*numLiterals+1) less than it is in 64 bits, and Fuel and other code must adjust things accordingly. See CompiledCode>>#initialPC

eliotmiranda commented 6 years ago

I said "Looks like..", not "Is" :-). This has to be verified. I could easily be wrong.

tinchodias commented 5 years ago

Hi. Some years ago we would solve this issue by signaling a fuel descriptive error. I mean, we would not add support to transform objects between 32 bits from/to 64 bits: "too smart" for Fuel, which was intended to just materialize what was serialized without conversions.

What do you think?

eliotmiranda commented 5 years ago

Hi Martin,

On Oct 10, 2018, at 12:09 PM, Martín Dias notifications@github.com wrote:

Hi. Some years ago we would solve this issue by signaling a fuel descriptive error. I mean, we would not add support to transform objects between 32 bits from/to 64 bits: "too smart" for Fuel, which was intended to just materialize what was serialized without conversions.

What do you think?

I know it is easy to convert between the two when deserializing. Squeak image segment support has already been updated to allow free interchange between 64 & 32 bits. And image segments are “physical”; tied to the heap representation. Fuel should be even easier, being a fully virtual format.

There are a few things to take care of:

Other than that, things tend to “just work” :-)

,,,^..^,,, (phone)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

theseion commented 3 years ago

This should now work (as of 3.0.3) but we'll have to test it. I don't have the means to do that at the moment.

seandenigris commented 3 years ago

Great! Let me see if I can find the data that first generated the error to test...

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will remain open but will probably not come into focus. If you still think this should receive some attention, leave a comment. Thank you for your contributions.

theseion commented 3 years ago

Still needs to be tested.

seandenigris commented 3 years ago

Seems to work (Ubuntu 16.04.3, Pharo 8.0):

Loaded Fuel into both 32 and 64 bit images via:

Metacello new
    repository: 'github://theseion/Fuel:3.0.2/repository';
    baseline: 'Fuel';
    load.

Then serialized in 32-bit image:

FLSerializer serialize: [ :ex | ex asNumber + 1 ] toFileNamed: '~/Documents/block-32-64/32/block32.fuel'

Then materialized in 64-bit image:

block := FLMaterializer materializeFromFileNamed: '~/Documents/block-32-64/32/block32.fuel'.
block value: '4' "=5"

I also did a second test returning the block from a method (instead of Playground) and it also worked as long as the method was in the target image as well.

Thanks!

theseion commented 3 years ago

Great! Thanks a lot!