On Monday 20 August 2007 16:49:53 Mark Glines wrote:
> With this patch, parrot rebuilds for me in 1 minute 41 seconds, plus or
> minus a few seconds. That's almost 80 seconds faster; a 45% improvement
> in build time.
>
> For reference, I also tried YAML::Syck, which built in 1 minute 55
> seconds. But Storable is in the perl base install, and its even faster.
>
> Eval sucks. Are there any drawbacks to this patch? The only thing I
> can think of is that its harder to read the .dump file now that it's
> binary. But it wasn't terribly well organized to begin with...
Good idea. I have just one suggestion:
> use_Storable_for_pmc_dump_files.diff
> Index: lib/Parrot/Pmc2c/Pmc2cMain.pm
> ===================================================================
> --- lib/Parrot/Pmc2c/Pmc2cMain.pm (revision 20738)
> +++ lib/Parrot/Pmc2c/Pmc2cMain.pm (working copy)
> @@ -248,9 +248,7 @@
> my ( $self, $filename ) = @_;
> $filename = $self->find_file( filename($filename, '.dump'), 1 );
>
> - my $class;
> - eval do { slurp($filename); };
> - die $@ if $@;
> + my $class = retrieve($filename) if -f $filename;
>
> return $class;
> }
my $foo if <something> creates a static-style variable, which could do the
wrong thing here. Another option is:
return retrieve($filename) if -f $filename;
... or:
my $class;
$class = retrieve($filename) if -f $filename;
return $class;
... or:
return unless -f $filename;
return retrieve($filename);
My preference is the latter, for clarity and context reasons.
-- c
