Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors #9

Merged
merged 19 commits into from Feb 23, 2019
Merged

Refactors #9

merged 19 commits into from Feb 23, 2019

Conversation

@yanick
Copy link
Contributor

@yanick yanick commented Nov 13, 2018

a bunch of small refactorings to make the code more perlish

@kaoru
Copy link
Owner

@kaoru kaoru commented Feb 23, 2019

Amazing, thanks @yanick!

I was slightly worried that a couple of the changes might bring a dip in performance. Specifically I think the use of a block eval in cadef79 might be slow, and I also thought that the single match regex before the substitution regexes that you removed in e230246 might've been for performance reasons.

Here's a dumb Dumbbench.pm script that shows they're about the same though...

#!/usr/bin/env perl

use strict;
use warnings;

use Dumbbench;

use YAML;
use Path::Tiny;
use Try::Tiny;
use Data::Visitor::Callback;

use Text::Hogan::Compiler;

my $bench = Dumbbench->new(
  target_rel_precision => 0.005, # seek ~0.5%
  initial_runs         => 20,    # the higher the more reliable
);

$bench->add_instances(
  Dumbbench::Instance::PerlSub->new(
    code => sub {
      for(1..10) {
        our $calls = 0;

        my $data_fixer = Data::Visitor::Callback->new(
          hash => sub {
            my ($self, $rh) = @_;

            # Handle lambdas in the ~lambdas.yml spec file
            for my $k (keys %$rh) {
              if ($k eq 'perl') {
                $_ = eval $rh->{'perl'};
                return;
              }
            }

            # Handle true/false values
            for my $v (values %$rh) {
              if ($v eq 'true') { $v = 1 }
              elsif ($v eq 'false') { $v = 0 }
              elsif (ref($v) eq 'HASH') { $self->visit($v) }
            }
          }
        );

        my @spec_files = path("t", "specs")->children(qr/[.]yml$/);

        for my $file (sort @spec_files) {
          my $yaml = $file->slurp_utf8;

          my $specs = YAML::Load($yaml);

          for my $test (@{ $specs->{tests} }) {
            #
            # Handle true/false values
            # Handle lambdas in the ~lambdas.yml spec file
            #
            $data_fixer->visit($test->{data});

            my $parser = Text::Hogan::Compiler->new();

            my $result = $parser->compile($test->{template})->render($test->{data}, $test->{partials});
            die $result unless $result eq $test->{'expected'};
          }
        }
      }
    }
  ),
);

$bench->run;
$bench->report;

Master branch:

Ran 32 iterations (7 outliers).
Rounded run time per iteration: 9.493e-01 +/- 1.2e-03 (0.1%)

Your branch:

Ran 37 iterations (6 outliers).
Rounded run time per iteration: 9.483e-01 +/- 1.8e-03 (0.2%)

Unfortunately I don't have a "real" app using Text::Hogan any more (we used it extensively at Nestoria before Mitula bought us in 2015 😅) so I can't test it out in production. If you are using it in a high traffic environment and you notice a slow down let me know and those will be the two places to look I think.

Merging! 👍 👍 1.09 on its way to CPAN 🚀

@kaoru kaoru merged commit 8f561c4 into kaoru:master Feb 23, 2019
@freyfogle
Copy link
Contributor

@freyfogle freyfogle commented Feb 23, 2019

we use Text::Hogan in Geo::Address::Formatter, will test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants