Re: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden)

[dak]

On 2018/07/05 21:32:25, Dan Eble wrote:
> On 2018/07/05 12:20:27, dak wrote:
> > I'd like to see some rationale for the amount of
semi-general-purpose tooling
> to
> > get there, and more current or future uses of it may go a long way
towards
> that.

> The rationale is that std::optional is fit for this situation and if
LilyPond
> were built with C++17 I would simply have used it.

Any C++17 lookalike package is _not_ "simply using it" but a maintenance
sink of itself.  And a semi-lookalike always requires double-checking in
what respects it does and does not do the same job.  Write code once,
debug it twice.  Not to mention profile it twice.

Reimplementing and then maintaining C++17 features and eventually
possibly phasing out and replacing them by "the real thing" in 4 years
or so is not the same as "simply using them".

> The interface of this
> Optional is a subset of std::optional, so when the project leaders
determine
> that it is reasonable to require C++17 to build LilyPond, discarding
this
> Optional should be straightforward.

Let me remind you that your project on "use std;" was hampered by us not
having been able to rid ourselves yet of std-vector.hh.

So I'd really like to see more of a possible use case than dimension
caching which could possibly just be done using a sentinel value (+nan.0
maybe?) or an explicit flag.  How many lines are we actually talking
about here?

The actual code using the dimension cache is just in grob.cc, and your
patch results in a line count change of +10, -17 here.  The way it
looks, you could likely even keep all of the grob.cc changes (the
const_cast looks icky though) and just wrap your interface into
dimension-cache without some semi-specialized variant of std::optional
if there is no obvious other use for it.  It can then still at some
future point of time conveniently be replaced with std::optional once we
consider that available.  And in the mean time we would not have to
think about a semi-implemented specialized substitute template class and
all the hassles that might bring along.

So do you see any other obvious candidates for std::optional we might
want to address using the generic mechanism of a template class, or
would we be better off leaving the cost of generalization for a future
time when we can just use the generalized solution that somebody else
has to bother with keeping in working order?

https://codereview.appspot.com/359770043/