Ticket #2167 (new RFC)

Opened 3 years ago

Last modified 3 years ago

Storable format breaks pmc2c when Perl is upgraded

Reported by: allison Owned by:
Priority: normal Milestone:
Component: pmc2c Version: 3.6.0
Severity: medium Keywords:
Cc: pmichaud, shockwave, cotto{{{ Language:
Patch status: Platform:

Description

Downstream in Debian, the upgrade from Perl 5.10 to 5.12 broke Rakudo compiles, because the installed PMC .dump files were written using the 5.10 version of Storable, and could not be read using the 5.12 version of Storable. The upstream bugs (closed) related to this failure are:

 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634105

 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634970

This is solved in the short-term, but in the longer-term, Storable is not a good format for installed, persistent metadata. (It was never intended to be installed, only to be a temporary build-process cache.)

We need to re-examine what metadata pmc2c actually needs for building PMCs that inherit from core Parrot PMCs. I suspect that it's actually much smaller than the current .dump files, and that installing some simple JSON metadata files for the core PMCs would actually be sufficient.

The work on M0 will eventually make this irrelevant, but that work would likely also benefit from some thought about abstracting the metadata needed to inherit from a PMC.

Attachments

installed_dump_sto_files.txt Download (3 bytes) - added by jkeenan 3 years ago.
94 .dump files in Storable format which get installed

Change History

  Changed 3 years ago by jkeenan

For reference, here are the places in the codebase where we will have to consider replacing use of Storable.

lib/Parrot/Pmc2c/PMC.pm:26:use Storable ();
lib/Parrot/Pmc2c/PMC.pm:74:    Storable::store( $self, $self->filename('.dump') );
lib/Parrot/Pmc2c/Pmc2cMain.pm:10:use Storable;
lib/Parrot/Pmc2c/Pmc2cMain.pm:142:
  consists of a binary dump of a hash reference, Storable-style.
lib/Parrot/Pmc2c/Pmc2cMain.pm:165:    return Storable::retrieve($filename);
lib/Parrot/Pmc2c/VTable.pm:6:use Storable ();
lib/Parrot/Pmc2c/VTable.pm:56:    Storable::store( $self, $dump_filename );

Changed 3 years ago by jkeenan

94 .dump files in Storable format which get installed

follow-up: ↓ 3   Changed 3 years ago by nick@…

On Sat, Jul 30, 2011 at 08:46:55AM -0000, Parrot wrote:

>  Downstream in Debian, the upgrade from Perl 5.10 to 5.12 broke Rakudo
>  compiles, because the installed PMC .dump files were written using the
>  5.10 version of Storable, and could not be read using the 5.12 version of
>  Storable. The upstream bugs (closed) related to this failure are:

I suspect that the problem is that they were written with Storable::store,
which is platform and configuration specific, and that when upgrading from
5.10 to 5.12 Debian changed their perl configuration from 32 to 64 bit
integers.

Storable itself is quite capable of reading files made all the way back to
0.60-something, for the same configuration.

Had parrot used Storable::nstore (the slower but portable format) there
would not have been this problem.

>  This is solved in the short-term, but in the longer-term, Storable is not
>  a good format for installed, persistent metadata. (It was never intended
>  to be installed, only to be a temporary build-process cache.)

However, that's key. As the intent was not to install the files, then
the faster "native" format was a sane choice.

And I agree with that (any) Storable format isn't an appropriate choice
for something installed. It's not a format that parrot itself can read.

Nicholas Clark

in reply to: ↑ 2   Changed 3 years ago by jkeenan

  • cc pmichaud, shockwave, cotto{{{ added

Replying to nick@…:

> I suspect that the problem is that they were written with Storable::store,
> which is platform and configuration specific, and that when upgrading from
> 5.10 to 5.12 Debian changed their perl configuration from 32 to 64 bit
> integers.
> 
> Storable itself is quite capable of reading files made all the way back to
> 0.60-something, for the same configuration.
> 
> Had parrot used Storable::nstore (the slower but portable format) there
> would not have been this problem.

I agree. For this reason we use Storable::nstore() in three other places in Parrot -- albeit all less mission-critical places in the configuration system:

lib/Parrot/Configure/Trace.pm
lib/Parrot/Configure.pm
lib/Parrot/Test/Pod.pm

I have created a tt2167/nstore branch in git which replaces store with nstore in lib/Parrot/Pmc2c/PMC.pm and lib/Parrot/Pmc2c/VTable.pm. Under an ordinary, non-optimized build on linux/i386, it passes make test  Smolder 19496. Although I agree that we should replace Storable as soon as possible, please give the branch a spin on the systems where we think we are having a problem.

Thank you very much.

kid51

  Changed 3 years ago by jkeenan

I have merged the tt2167/nstore branch into master in c70c89c, so we're now using nstore instead of store throughout. This improves things but does not address the long-term problem.

Thank you very much.

kid51

Note: See TracTickets for help on using tickets.