From: Peter Rabbitson Date: Sun, 21 Sep 2008 17:36:58 +0000 (+0000) Subject: Revert r482[45], by implementing a better version of r4760 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=a5b293612996cda25ce7e7bf1a5a5a23249c7b01;p=dbsrgits%2FDBIx-Class-Historic.git Revert r482[45], by implementing a better version of r4760 --- diff --git a/Changes b/Changes index cecc10f..339f0fd 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,10 @@ Revision history for DBIx::Class path across multiple versions (jgoulah) - Better (and marginally faster) implementation of the HashRefInflator hash construction algorithm + - Added the ability to instantiate HashRefInflator so options can be + passed to the constructor + - Additional recursive function to optionally inflate any inflatable + values in the hashref generated by HashRefInflator - Allow explicit specification of ON DELETE/ON UPDATE constraints when using the SQLT parser diff --git a/lib/DBIx/Class/Manual/Cookbook.pod b/lib/DBIx/Class/Manual/Cookbook.pod index 83a633d..6717ea7 100644 --- a/lib/DBIx/Class/Manual/Cookbook.pod +++ b/lib/DBIx/Class/Manual/Cookbook.pod @@ -733,7 +733,7 @@ B test.pl ### The statement below will print print "I can do admin stuff\n" if $admin->can('do_admin_stuff'); -=head2 Skip object creation for faster results +=head2 Skip row object creation for faster results DBIx::Class is not built for speed, it's built for convenience and ease of use, but sometimes you just need to get the data, and skip the @@ -746,9 +746,20 @@ To do this simply use L. $rs->result_class('DBIx::Class::ResultClass::HashRefInflator'); my $hash_ref = $rs->find(1); - + Wasn't that easy? +=head2 Skip row object creation for faster results, but still inflate +column values to the corresponding objects + + my $rs = $schema->resultset('CD'); + + $rs->result_class(DBIx::Class::ResultClass::HashRefInflator->new ( + inflate_columns => 1 + )); + + my $hash_ref = $rs->find(1); + =head2 Get raw data for blindingly fast results If the L solution diff --git a/lib/DBIx/Class/ResultClass/HashRefInflator.pm b/lib/DBIx/Class/ResultClass/HashRefInflator.pm index 945bcfd..9bbf65b 100644 --- a/lib/DBIx/Class/ResultClass/HashRefInflator.pm +++ b/lib/DBIx/Class/ResultClass/HashRefInflator.pm @@ -9,79 +9,78 @@ DBIx::Class::ResultClass::HashRefInflator =head1 SYNOPSIS + use DBIx::Class::ResultClass::HashRefInflator; + my $rs = $schema->resultset('CD'); $rs->result_class('DBIx::Class::ResultClass::HashRefInflator'); + or + $rs->result_class(DBIx::Class::ResultClass::HashRefInflator->new (%args)); + + while (my $hashref = $rs->next) { + ... + } =head1 DESCRIPTION -DBIx::Class is not built for speed: it's built for convenience and -ease of use. But sometimes you just need to get the data, and skip the -fancy objects. That is what this class provides. +DBIx::Class is faster than older ORMs like Class::DBI but it still isn't +designed primarily for speed. Sometimes you need to quickly retrieve the data +from a massive resultset, while skipping the creation of fancy row objects. +Specifying this class as a C for a resultset will change C<< $rs->next >> +to return a plain data hash-ref (or a list of such hash-refs if C<< $rs->all >> is used). -There are two ways of using this class. +There are two ways of using this class: =over =item * -Specify C<< $rs->result_class >> on a specific resultset to affect only that -resultset (and any chained off of it); or +Supply an instance of DBIx::Class::ResultClass::HashRefInflator to +C<< $rs->result_class >>. See L for a list of valid +arguments to new(). =item * -Specify C<< __PACKAGE__->result_class >> on your source object to force all -uses of that result source to be inflated to hash-refs - this approach is not -recommended. +Another way is to simply supply the class name as a string to +C<< $rs->result_class >>. Equivalent to passing +DBIx::Class::ResultClass::HashRefInflator->new(). =back -=head1 AUTOMATICALLY INFLATING COLUMN VALUES - -So you want to skip the DBIx::Class object creation part, but you still want -all your data to be inflated according to the rules you defined in your table -classes. Setting the global variable -C<$DBIx::Class::ResultClass::HashRefInflator::inflate_data> to a true value -will instruct L to interrogate the processed columns and apply any -inflation methods declared via L. - -For increased speed the inflation method lookups are cached in -C<%DBIx::Class::ResultClass::HashRefInflator::inflator_cache>. Make sure to -reset this hash if you modify column inflators at run time. +There are two ways of applying this class to a resultset: -=head1 METHODS - -=head2 inflate_result - -Inflates the result and prefetched data into a hash-ref using L. +=over -=cut +=item * -sub inflate_result { - my ($self, $source, $me, $prefetch) = @_; +Specify C<< $rs->result_class >> on a specific resultset to affect only that +resultset (and any chained off of it); or - my $hashref = mk_hash($me, $prefetch); - return $hashref; -} +=item * -=head2 mk_hash +Specify C<< __PACKAGE__->result_class >> on your source object to force all +uses of that result source to be inflated to hash-refs - this approach is not +recommended. -This does all the work of inflating the (pre)fetched data. +=back =cut ############## # NOTE # -# Generally people use this to gain as much speed as possible. If a new mk_hash is +# Generally people use this to gain as much speed as possible. If a new &mk_hash is # implemented, it should be benchmarked using the maint/benchmark_hashrefinflator.pl -# script (in addition to passing all tests of course :). Additional instructions are +# script (in addition to passing all tests of course :). Additional instructions are # provided in the script itself. # -sub mk_hash { +# This coderef is a simple recursive function +# Arguments: ($me, $prefetch) from inflate_result() below +my $mk_hash; +$mk_hash = sub { if (ref $_[0] eq 'ARRAY') { # multi relationship - return [ map { mk_hash (@$_) || () } (@_) ]; + return [ map { $mk_hash->(@$_) || () } (@_) ]; } else { my $hash = { @@ -90,7 +89,7 @@ sub mk_hash { # the second arg is a hash of arrays for each prefetched relation map - { $_ => mk_hash( @{$_[1]->{$_}} ) } + { $_ => $mk_hash->( @{$_[1]->{$_}} ) } ( $_[1] ? (keys %{$_[1]}) : () ) }; @@ -102,9 +101,101 @@ sub mk_hash { return undef; } +}; + +# This is the inflator +my $inflate_hash; +$inflate_hash = sub { + my ($hri_instance, $schema, $rc, $data) = @_; + + foreach my $column (keys %{$data}) { + + if (ref $data->{$column} eq 'HASH') { + $inflate_hash->($hri_instance, $schema, $schema->source ($rc)->related_class ($column), $data->{$column}); + } + elsif (ref $data->{$column} eq 'ARRAY') { + foreach my $rel (@{$data->{$column}}) { + $inflate_hash->($hri_instance, $schema, $schema->source ($rc)->related_class ($column), $rel); + } + } + else { + # "null is null is null" + next if not defined $data->{$column}; + + # cache the inflator coderef + unless (exists $hri_instance->{_inflator_cache}{$rc}{$column}) { + $hri_instance->{_inflator_cache}{$rc}{$column} = exists $schema->source ($rc)->_relationships->{$column} + ? undef # currently no way to inflate a column sharing a name with a rel + : $rc->column_info($column)->{_inflate_info}{inflate} + ; + } + + if ($hri_instance->{_inflator_cache}{$rc}{$column}) { + $data->{$column} = $hri_instance->{_inflator_cache}{$rc}{$column}->($data->{$column}); + } + } + } +}; + + +=head1 METHODS + +=head2 new + + $class->new( %args ); + $class->new({ %args }); + +Creates a new DBIx::Class::ResultClass::HashRefInflator object. Takes the following +arguments: + +=over + +=item inflate_columns + +Sometimes you still want all your data to be inflated to the corresponding +objects according to the rules you defined in your table classes (e.g. you +want all dates in the resulting hash to be replaced with the equivalent +DateTime objects). Supplying C<< inflate_columns => 1 >> to the constructor will +interrogate the processed columns and apply any inflation methods declared +via L to the contents of the +resulting hash-ref. + +=back + +=cut + +sub new { + my $self = shift; + my $args = { (ref $_[0] eq 'HASH') ? %{$_[0]} : @_ }; + return bless ($args, $self) +} + +=head2 inflate_result + +Inflates the result and prefetched data into a hash-ref (invoked by L) + +=cut + + +sub inflate_result { + my ($self, $source, $me, $prefetch) = @_; + + my $hashref = $mk_hash->($me, $prefetch); + + # if $self is an instance and inflate_columns is set + if ( (ref $self) and $self->{inflate_columns} ) { + $inflate_hash->($self, $source->schema, $source->result_class, $hashref); + } + + return $hashref; } -=head1 CAVEAT + +=head1 CAVEATS + +=over + +=item * This will not work for relationships that have been prefetched. Consider the following: @@ -119,6 +210,14 @@ C<$first> will B be a hashref, it will be a normal CD row since HashRefInflator only affects resultsets at inflation time, and prefetch causes relations to be inflated when the master C<$artist> row is inflated. +=item * + +When using C, the inflation method lookups are cached in the +HashRefInflator object for additional speed. If you modify column inflators at run +time, make sure to grab a new instance of this class to avoid cached surprises. + +=back + =cut 1; diff --git a/maint/benchmark_hashrefinflator.pl b/maint/benchmark_hashrefinflator.pl index d8dd947..5761051 100755 --- a/maint/benchmark_hashrefinflator.pl +++ b/maint/benchmark_hashrefinflator.pl @@ -1,56 +1,45 @@ #!/usr/bin/perl -use warnings; -use strict; - -use FindBin; - # # So you wrote a new mk_hash implementation which passed all tests (particularly # t/68inflate_resultclass_hashrefinflator) and would like to see how it holds up -# against older versions of the same. Just add your subroutine somewhere below and -# add its name to the @bench array. Happy testing. - -my @bench = qw/current_mk_hash old_mk_hash/; +# against older versions of the same. Just add your coderef to the HRI::Bench +# namespace and add a name/ref pair to the %bench_list hash. Happy testing. -use Benchmark qw/timethis cmpthese/; - -use lib ("$FindBin::Bin/../lib", "$FindBin::Bin/../t/lib"); -use DBICTest; -use DBIx::Class::ResultClass::HashRefInflator; - -chdir ("$FindBin::Bin/.."); -my $schema = DBICTest->init_schema(); - -my $test_sub = sub { - my $rs_hashrefinf = $schema->resultset ('Artist')->search ({}, { - prefetch => { cds => 'tracks' }, - }); - $rs_hashrefinf->result_class('DBIx::Class::ResultClass::HashRefInflator'); - my @stuff = $rs_hashrefinf->all; -}; +package DBIx::Class::ResultClass::HashRefInflator::Bench; +use warnings; +use strict; -my $results; -for my $b (@bench) { - die "No such subroutine '$b' defined!\n" if not __PACKAGE__->can ($b); - print "Timing $b... "; +my $current_mk_hash; +$current_mk_hash = sub { + if (ref $_[0] eq 'ARRAY') { # multi relationship + return [ map { $current_mk_hash->(@$_) || () } (@_) ]; + } + else { + my $hash = { + # the main hash could be an undef if we are processing a skipped-over join + $_[0] ? %{$_[0]} : (), - # switch the inflator - no warnings qw/redefine/; - no strict qw/refs/; - local *DBIx::Class::ResultClass::HashRefInflator::mk_hash = \&$b; + # the second arg is a hash of arrays for each prefetched relation + map + { $_ => $current_mk_hash->( @{$_[1]->{$_}} ) } + ( $_[1] ? (keys %{$_[1]}) : () ) + }; - $results->{$b} = timethis (-2, $test_sub); -} -cmpthese ($results); + # if there is at least one defined column consider the resultset real + # (and not an emtpy has_many rel containing one empty hashref) + for (values %$hash) { + return $hash if defined $_; + } -#----------------------------- -# mk_hash implementations -#----------------------------- + return undef; + } +}; # the (incomplete, fails a test) implementation before svn:4760 -sub old_mk_hash { +my $old_mk_hash; +$old_mk_hash = sub { my ($me, $rest) = @_; # $me is the hashref of cols/data from the immediate resultsource @@ -74,35 +63,56 @@ sub old_mk_hash { map { ( $_ => ref($rest->{$_}[0]) eq 'ARRAY' - ? [ grep defined, map old_mk_hash(@$_), @{$rest->{$_}} ] - : old_mk_hash( @{$rest->{$_}} ) + ? [ grep defined, map $old_mk_hash->(@$_), @{$rest->{$_}} ] + : $old_mk_hash->( @{$rest->{$_}} ) ) } keys %$rest }; -} +}; -# current implementation as of svn:4760 -sub current_mk_hash { - if (ref $_[0] eq 'ARRAY') { # multi relationship - return [ map { current_mk_hash (@$_) || () } (@_) ]; - } - else { - my $hash = { - # the main hash could be an undef if we are processing a skipped-over join - $_[0] ? %{$_[0]} : (), - # the second arg is a hash of arrays for each prefetched relation - map - { $_ => current_mk_hash( @{$_[1]->{$_}} ) } - ( $_[1] ? (keys %{$_[1]}) : () ) - }; +our %bench_list = ( + current_implementation => $current_mk_hash, + old_implementation => $old_mk_hash, +); - # if there is at least one defined column consider the resultset real - # (and not an emtpy has_many rel containing one empty hashref) - for (values %$hash) { - return $hash if defined $_; - } +1; - return undef; - } +package benchmark_hashrefinflator; + +use warnings; +use strict; + +use FindBin; +use lib ("$FindBin::Bin/../lib", "$FindBin::Bin/../t/lib"); + +use Benchmark qw/timethis cmpthese/; +use DBICTest; + +chdir ("$FindBin::Bin/.."); +my $schema = DBICTest->init_schema(); + +my $test_sub = sub { + my $rs_hashrefinf = $schema->resultset ('Artist')->search ({}, { + prefetch => { cds => 'tracks' }, + }); + $rs_hashrefinf->result_class('DBIx::Class::ResultClass::HashRefInflator::Bench'); + my @stuff = $rs_hashrefinf->all; +}; + + +my $results; +for my $b (keys %DBIx::Class::ResultClass::HashRefInflator::Bench::bench_list) { + + print "Timing $b... "; + + # switch the inflator + no warnings qw/redefine once/; + no strict qw/refs/; + local *DBIx::Class::ResultClass::HashRefInflator::Bench::inflate_result = sub { + return $DBIx::Class::ResultClass::HashRefInflator::Bench::bench_list{$b}->(@_[2,3]); + }; + + $results->{$b} = timethis (-2, $test_sub); } +cmpthese ($results); diff --git a/t/68inflate_resultclass_hashrefinflator.t b/t/68inflate_resultclass_hashrefinflator.t index a0402a1..e8fe25b 100644 --- a/t/68inflate_resultclass_hashrefinflator.t +++ b/t/68inflate_resultclass_hashrefinflator.t @@ -120,6 +120,12 @@ for my $index (0 .. $#hashrefinf) { # Test the data inflator +is_deeply ( + DBIx::Class::ResultClass::HashRefInflator->new (inflate_columns => 1), + DBIx::Class::ResultClass::HashRefInflator->new ({inflate_columns => 1}), + 'Make sure arguments as list and as hashref work identically' +); + $schema->class('CD')->inflate_column( 'year', { inflate => sub { DateTime->new( year => shift ) }, deflate => sub { shift->year } } @@ -131,3 +137,16 @@ $cd_rs->result_class('DBIx::Class::ResultClass::HashRefInflator'); my $cd = $cd_rs->first; ok ( (not blessed $cd->{year}), "Plain string returned for year"); is ( $cd->{year}, '1997', "We are looking at the right year"); + +# try again with a HRI instance +$cd_rs->reset; +$cd_rs->result_class(DBIx::Class::ResultClass::HashRefInflator->new); +my $cd2 = $cd_rs->first; +is_deeply ($cd, $cd2, "HRI used as instance returns the same hashref as the old result_class ('class')"); + +# try it again with inflation requested +$cd_rs->reset; +$cd_rs->result_class(DBIx::Class::ResultClass::HashRefInflator->new (inflate_columns => 1)); +my $cd3 = $cd_rs->first; +isa_ok ($cd3->{year}, 'DateTime', "Inflated object"); +is ($cd3->{year}, DateTime->new ( year => 1997 ), "Correct year was inflated");