[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 3597
From: |
dak |
Subject: |
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden) |
Date: |
Sat, 07 Jul 2018 08:13:19 -0700 |
Still LGTM.
https://codereview.appspot.com/359770043/diff/20001/lily/grob.cc
File lily/grob.cc (left):
https://codereview.appspot.com/359770043/diff/20001/lily/grob.cc#oldcode325
lily/grob.cc:325: *dim_cache_[a].offset_ += y;
On 2018/07/07 14:48:27, Dan Eble wrote:
On 2018/07/07 11:06:11, dak wrote:
> This is more an "as you like it" suggestion: operator priority of *
as opposed
> to [] . -> is not always clear to everybody, so
*(dim_cache_[a].offset_) += y;
> might be a consideration. On the other hand, it's ugly. And it's
not like
I would have no problem adding parentheses, but how do you rate the
beauty of
using value_or() here as in Patch Set 1?
Reads clumsy to me, though it's basically the same as our robust_scm2* .
Let's just leave this one alone for now.
https://codereview.appspot.com/359770043/diff/20001/lily/grob.cc
File lily/grob.cc (right):
https://codereview.appspot.com/359770043/diff/20001/lily/grob.cc#newcode322
lily/grob.cc:322: if (!dim_cache_[a].offset_.has_value ())
On 2018/07/07 14:48:27, Dan Eble wrote:
Well, even bool x = 0.0 would cause me to question the state of mind
of the
programmer.
lily/bar-check-iterator.cc: if (where->main_part_)
Pretty much the same. We are not using all that many inexact numbers in
a similar manner I guess but numerics used as conditions are not that
infrequent although it's admittedly more integers (like size ()) used in
that manner.
https://codereview.appspot.com/359770043/diff/20001/lily/include/dimension-cache.hh
File lily/include/dimension-cache.hh (right):
https://codereview.appspot.com/359770043/diff/20001/lily/include/dimension-cache.hh#newcode72
lily/include/dimension-cache.hh:72: void clear ()
On 2018/07/07 14:48:27, Dan Eble wrote:
It was already called clear () and I was trying not to change more
than I had
to.
In this case I don't really like either of those names
So just not wanting to condone either and not touching anything.
Ok with me.
https://codereview.appspot.com/359770043/
- Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden), (continued)
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden), nine . fierce . ballads, 2018/07/06
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden), nine . fierce . ballads, 2018/07/06
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden), nine . fierce . ballads, 2018/07/06
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden), dak, 2018/07/07
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden), nine . fierce . ballads, 2018/07/07
Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden),
dak <=