ingydotnet / yaml-libyaml-pm

Perl Binding to libyaml
http://search.cpan.org/dist/YAML-LibYAML/
33 stars 37 forks source link

libyaml-libyaml-perl: [PATCH] lazy load B::Deparse for big speedup #52

Closed carnil closed 7 years ago

carnil commented 7 years ago

We have the following bug reported to the Debian package of YAML-LibYAML (https://bugs.debian.org/849843):

------8<-----------8<-----------8<-----------8<-----------8<-----

Package: libyaml-libyaml-perl
Version: 0.63-1
Severity: normal
Tags: patch upstream

This speeds up startup time dramatically and is noticeable with
small scripts on my weak hardware where loading B::Deparse takes
a significant amount of time.  Lazy loading it helps for the
common case when I do not need to dump a coderef.

"require" is idempotent in Perl, so it has virtually no overhead
across repeated invocations of coderef2text.

With this patch, the following script runs in around 30ms
without any args passed; and around 150ms with "code" as the
first arg.  Without the patch, it takes around 150ms regardless.

-----------8<-----------
use YAML::XS qw(Dump);
my $h = { a => 'b', c => 'd' };
warn Dump($h);
if (shift eq 'code') {
    warn $YAML::XS::coderef2text->(sub { print "Hi\n" });
}
-----------8<-----------

Note: I realize upstream has a git repository and bug tracker;
but I will not use that as that requires registering on a site
which is non-Free Software, requires non-Free JavaScript (which
I wouldn't run regardless given my slow hardware) and has an
unacceptable terms-of-service.  So, thank you to Debian
for continuing to host an open-to-all bug tracker :)

------8<-----------8<-----------8<-----------8<-----------8<-----

The patch is

>From 8c51950fa3796346542ca01ee04659edaaf81cc3 Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Sun, 1 Jan 2017 08:05:59 +0000
Subject: [PATCH] load B::Deparse lazily

This speeds up startup time dramatically and is noticeable with
small scripts on my weak hardware where loading B::Deparse takes
a significant amount of time.  Lazy loading it helps for the
common case when I do not need to dump a coderef.

"require" is idempotent in Perl, so it has virtually no overhead
across repeated invocations of coderef2text.

With this patch, the following script runs in around 30ms
without any args passed; and around 150ms with "code" as the
first arg.  Without the patch, it takes around 150ms regardless.

-----------8<-----------
use YAML::XS qw(Dump);
my $h = { a => 'b', c => 'd' };
warn Dump($h);
if (shift eq 'code') {
        warn $YAML::XS::coderef2text->(sub { print "Hi\n" });
}
-----------8<-----------
---
 lib/YAML/XS.pm | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/YAML/XS.pm b/lib/YAML/XS.pm
index 90253c8..0fd2df4 100644
--- a/lib/YAML/XS.pm
+++ b/lib/YAML/XS.pm
@@ -50,15 +50,11 @@ sub LoadFile {
     return YAML::XS::LibYAML::Load(do { local $/; local $_ = <$IN> });
 }

-# XXX Figure out how to lazily load this module.
-# So far I've tried using the C function:
-#      load_module(PERL_LOADMOD_NOIMPORT, newSVpv("B::Deparse", 0), NULL);
-# But it didn't seem to work.
-use B::Deparse;

 # XXX The following code should be moved from Perl to C.
 $YAML::XS::coderef2text = sub {
     my $coderef = shift;
+    require B::Deparse;
     my $deparse = B::Deparse->new();
     my $text;
     eval {
--
EW

Thanks for considering, Salvatore Bonaccorso, Debian Perl Group

perlpunk commented 7 years ago

thanks! good idea, we did the same change in YAML.pm recently

ingydotnet commented 7 years ago

@carnil This has been released to CPAN as YAML-LibYAML-0.63_001

If all goes well testing-wise, we'll release 0.64 in the next few days.

Thanks.