lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Issue 5368: Reduce allocations in Grob dimension caching (issue 35977004


From: dak
Subject: Issue 5368: Reduce allocations in Grob dimension caching (issue 359770043 by address@hidden)
Date: Thu, 05 Jul 2018 05:20:27 -0700

Overall, I consider the problem you are trying to address here real:
allocation of the relevant values seems like completely overblown and it
does appear that the allocation/indirection is mainly being used in lieu
of a flag.

The "boring" way for that would be just to actually add the flag in
question.

Can you line out other use cases here that would warrant the complexity
of dragging in a mechanism like this?

In its current version, it does not work satisfiably with types
involving non-trivial construction/destruction.  That makes it reek of a
single-use class and then it seems hard to justify all the expense over
just using explicit addition of a flag and removal of the indirection.

So what additional uses would you consider this useful for?

The actual objective of not actually allocating memory for the caches
here is valid, and the resulting assembly code should likely be similar
to what gets likely produced by this patch.

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.


https://codereview.appspot.com/359770043/diff/1/flower/include/optional.hh
File flower/include/optional.hh (right):

https://codereview.appspot.com/359770043/diff/1/flower/include/optional.hh#newcode30
flower/include/optional.hh:30: // type at all times, even when it
presents itself as empty.
The way against that would be to have
char [sizeof (T)] value_space_;
and instantiate and destroy it with placement new syntax, right?

You are currently replacing optional Real values where
construction/deletion is not really relevant.  Should we try
anticipating other uses?

https://codereview.appspot.com/359770043/diff/1/flower/include/optional.hh#newcode40
flower/include/optional.hh:40: typedef const T Optional::*bool_type;
I don't think we should be dragging an idiom like that into a single
class/template like this.

In particular when it comes with additional baggage like the comparison
operator business in order to actually work as intended.

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]