https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc
File flower/rational.cc (right):
https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode257
flower/rational.cc:257: if (is_infinity ())
On 2020/04/12 16:55:05, Dan Eble wrote:
> The hazard of reviewing this class is that there's always something to
wonder
> about. This time, I wonder about adding +infinity and -infinity
together (in
> either order).
yes. I recommend against doing that :-)
https://codereview.appspot.com/551690046/diff/573690043/flower/rational.cc#newcode261
flower/rational.cc:261: else if (r.sign_ == 0)
On 2020/04/12 16:55:05, Dan Eble wrote:
> Is this the only micro-optimization you measured? Other ideas you
might wish to
> try:
>
> * move the conditions that read this->sign_ together
>
> * are infinities handled first because they are significantly more
common than
> zeros? try moving the branches that test for zero before the branches
that test
> for infinities
>
> * inline a subset of the shortcuts in rational.hh
>
> In any case, noting your findings in a concise comment would help
future
> maintainers avoid erasing your gains.
It's the only one I investigated. The rational addition shows up
because we add Moments together in many places. Since those generally
have grace == 0, it's doing a lot of unwarranted
multiplication/division.
I'll move the zero check first, because it's more likely than the
infinity check.
https://codereview.appspot.com/551690046/
