Skip to content

Commit

Permalink
Make infix:<**>(Rational, Int) more than 10x as fast
Browse files Browse the repository at this point in the history 
Makes `<50000123/50000000> ** 50_000` run 10.26x as fast
Makes `for ^2000 { my $ = (42 / $_) ** $_ }` run 16.72x as fast
Closes #1955 R#1955

Rationals are now[^1] guaranteed to be always in reduced form,
so the numerator and the denominator cannot have prime factors in common,
and raising each of them to a power doesn't change that, so we
can omit trying to reduce the result.

[1] 5c429e4
  • Loading branch information
@zoffixznet
zoffixznet committed Jul 31, 2018
1 parent 38de1e5 commit d1729da
Showing 1 changed file with 36 additions and 22 deletions.
58 changes: 36 additions & 22 deletions src/core/Rat.pm6
View file
Expand Up @@ -46,19 +46,23 @@ sub DIVIDE_NUMBERS(Int:D \nu, Int:D \de, \t1, \t2) {
nqp::stmts(
($numerator := nqp::neg_I($numerator, Int)),
($denominator := nqp::neg_I($denominator, Int)))),
RAKUDO_INTERNAL_DIVIDE_NUMBERS_NO_NORMALIZE $numerator, $denominator, t1, t2)
}

sub RAKUDO_INTERNAL_DIVIDE_NUMBERS_NO_NORMALIZE(\nu, \de, \t1, \t2) {
nqp::if(
nqp::istype(t1, FatRat) || nqp::istype(t2, FatRat),
nqp::p6bindattrinvres(
nqp::p6bindattrinvres(nqp::create(FatRat),
FatRat,'$!numerator',nu),
FatRat,'$!denominator',de),
nqp::if(
nqp::istype(t1, FatRat) || nqp::istype(t2, FatRat),
nqp::islt_I(de, UINT64_UPPER),
nqp::p6bindattrinvres(
nqp::p6bindattrinvres(nqp::create(FatRat),
FatRat,'$!numerator',$numerator),
FatRat,'$!denominator',$denominator),
nqp::if(
nqp::islt_I($denominator, UINT64_UPPER),
nqp::p6bindattrinvres(
nqp::p6bindattrinvres(nqp::create(Rat),
Rat,'$!numerator',$numerator),
Rat,'$!denominator',$denominator),
nqp::p6box_n(nqp::div_In($numerator, $denominator)))))
nqp::p6bindattrinvres(nqp::create(Rat),
Rat,'$!numerator',nu),
Rat,'$!denominator',de),
nqp::p6box_n(nqp::div_In(nu, de))))
}

multi sub prefix:<->(Rat:D \a) {
Expand Down Expand Up @@ -179,17 +183,27 @@ multi sub infix:<%>(Rational:D \a, Rational:D \b) {
}

multi sub infix:<**>(Rational:D \a, Int:D \b) {
b >= 0
?? DIVIDE_NUMBERS
(a.numerator ** b // fail (a.numerator.abs > a.denominator ?? X::Numeric::Overflow !! X::Numeric::Underflow).new),
a.denominator ** b, # we presume it likely already blew up on the numerator
a,
b
!! DIVIDE_NUMBERS
(a.denominator ** -b // fail (a.numerator.abs < a.denominator ?? X::Numeric::Overflow !! X::Numeric::Underflow).new),
a.numerator ** -b,
a,
b
my $nu;
my $de;
nqp::if(
nqp::isge_I(nqp::decont(b), 0),
nqp::if( # if we got Inf
nqp::istype(($nu := nqp::pow_I(a.numerator, nqp::decont(b), Num, Int)), Num),
Failure.new(X::Numeric::Overflow.new),
nqp::if( # if we got Inf
nqp::istype(($de := nqp::pow_I(a.denominator, nqp::decont(b), Num, Int)), Num),
Failure.new(X::Numeric::Overflow.new),
RAKUDO_INTERNAL_DIVIDE_NUMBERS_NO_NORMALIZE $nu, $de, a, b)),
nqp::if( # if we got 0 as result, but shouldn't have
nqp::isfalse($nu := nqp::pow_I(a.numerator, nqp::neg_I(nqp::decont(b), Int), Num, Int))
&& a.numerator,
Failure.new(X::Numeric::Underflow.new),
nqp::if( # if we got 0 as result, but shouldn't have
nqp::isfalse($de := nqp::pow_I(a.denominator,
nqp::neg_I(nqp::decont(b), Int), Num, Int))
&& a.denominator,
Failure.new(X::Numeric::Underflow.new),
RAKUDO_INTERNAL_DIVIDE_NUMBERS_NO_NORMALIZE $de, $nu, a, b)))
}

multi sub infix:<==>(Rational:D \a, Rational:D \b) {
Expand Down

3 comments on commit d1729da

@zoffixznet
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to mention in the commit:

Also fixes crash in constructs like <1/50000000000000> ** 5000000000000 because
the original code makes incorrect assumption: we presume it likely already blew up on the numerator

With that, I also tweaked the thrown exceptions: if power is positive, we now always throw Overflow (and always Underflow for negative), as that's the actual reason we fail.

@b2gills
Copy link
Contributor

@b2gills b2gills commented on d1729da Aug 1, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these both produce the same failure?

> <1/50000000000000> ** 5000000000000
Numeric overflow
  in block <unit> at <unknown file> line 1

> <1/50000000000000>.Num ** 5000000000000
Numeric underflow
  in block <unit> at <unknown file> line 1

@zoffixznet
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these both produce the same failure?

I can argue both ways:

  1. If you view <1/50000000000000> as a single number (like you're doing with .Num), that you're then raising to a power and by doing so making it smaller, then you get an Underflow
  2. (the one I went with) If you view <1/50000000000000> as a Rat object that has a numerator and denominator, that you're raising to a power and receiving a Rat object with raised numerator/denominator, then you get an Overflow, because the raising of denominator to that power caused a numeric overflow. In other words, I'm not actually performing any division by requesting a new Rat object, so nothing is underflowing; similar to how ((1/0)**42) just produces a Rat object and isn't a division-by-zero explosion.

Please sign in to comment.