Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix .classify assuming side effects
We now make sure to only evaluate &test once for each element of the input list.
Fixes RT #126032

This new implementation takes an Iterable instead of a List giving a speedup
of 15 % for (1..500000).classify(* % 5)
  • Loading branch information
niner committed Sep 14, 2015
1 parent 426ade4 commit b4a9f56
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/core/Any.pm
Expand Up @@ -117,10 +117,10 @@ my class Any { # declared in BOOTSTRAP
die "Doesn't make sense to classify with itself";
}
multi method classify($test, :$into!, :&as) {
( $into // $into.new ).classify-list( $test, self.list, :&as);
( $into // $into.new ).classify-list( $test, self, :&as);
}
multi method classify($test, :&as) {
Hash.^parameterize(Any,Any).new.classify-list( $test, self.list, :&as );
Hash.^parameterize(Any,Any).new.classify-list( $test, self, :&as );
}

proto method categorize(|) is nodal { * }
Expand Down
46 changes: 27 additions & 19 deletions src/core/Hash.pm
Expand Up @@ -143,43 +143,51 @@ my class Hash { # declared in BOOTSTRAP
}

proto method classify-list(|) { * }
# XXX GLR possibly more efficient taking an Iterable, not a @list
multi method classify-list( &test, @list, :&as ) {
fail X::Cannot::Lazy.new(:action<classify>) if @list.is-lazy;
if @list {
multi method classify-list( &test, \list, :&as ) {
fail X::Cannot::Lazy.new(:action<classify>) if list.is-lazy;
my \iter = (nqp::istype(list, Iterable) ?? list !! list.list).iterator;
my $value := iter.pull-one;
unless $value =:= IterationEnd {
my $tested := test($value);

# multi-level classify
if nqp::istype(test(@list[0]),Iterable) {
@list.map: -> $l {
my @keys = test($l);
if nqp::istype($tested, Iterable) {
loop {
my @keys = $tested;
my $last := @keys.pop;
my $hash = self;
$hash = $hash{$_} //= self.new for @keys;
nqp::push(
nqp::getattr(nqp::decont($hash{$last} //= []), List, '$!reified'),
&as ?? as($l) !! $l
&as ?? as($value) !! $value
);
}
last if ($value := iter.pull-one) =:= IterationEnd;
$tested := test($value);
};
}

# simple classify to store a specific value
elsif &as {
@list.map: {
loop {
nqp::push(
nqp::getattr(nqp::decont(self{test $_} //= []), List, '$!reified'),
as($_)
)
}
nqp::getattr(nqp::decont(self{$tested} //= []), List, '$!reified'),
as($value)
);
last if ($value := iter.pull-one) =:= IterationEnd;
$tested := test($value);
};
}

# just a simple classify
else {
@list.map: {
loop {
nqp::push(
nqp::getattr(nqp::decont(self{test $_} //= []), List, '$!reified'),
$_
)
}
nqp::getattr(nqp::decont(self{$tested} //= []), List, '$!reified'),
$value
);
last if ($value := iter.pull-one) =:= IterationEnd;
$tested := test($value);
};
}
}
self;
Expand Down

0 comments on commit b4a9f56

Please sign in to comment.