Re: Shortcut Rational addition if either operand is zero (issue 551690046 by address@hidden)

[hanwenn]

I pushed this to staging by accident; sorry.

I apparently have no permission to do a force-push.


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#newcode261
flower/rational.cc:261: else if (r.sign_ == 0)
On 2020/04/12 18:01:31, hanwenn wrote:
> 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.

Done.

https://codereview.appspot.com/551690046/