[PATCH] D34838: Prototype: Reduce llvm-profdata merge memory usage further

[David Blaikie via llvm-commits]

On Thu, Jun 29, 2017 at 5:18 PM David Li via Phabricator <
reviews at reviews.llvm.org> wrote:

> davidxl added a comment.
>
> Instead of exposing Counter members widely (and fixing tests), it is
> better to add wrapper methods in InstrProfRecord to forward the call to the
> nested member struct.
>

Seems like a lot of wrappers - is that the ideal API going forward, or only
to minimize changes made by this patch? (I've already done the work & LLVM
as a community's usually fine with the churn such that minimizing change
isn't too important if it seems like the way things should be)

Though I admit this is a bit so-so/uncertain. One pro is that I think
lacking the wrappers encourages APIs to only depend on the part of the
structure they need (in fact on that basis I'd consider sinking many of the
value-profiling based functions into a separate class wrapping the
unique_ptr) rather than the whole thing, making their contract clearer
("this function is only dealing with value profiling data", "this function
cares about the counters as well", "this function needs names, etc") & more
flexible (you can call it when you only have that part/chunk/thing, not
needing the whole thing around).


>
>
> https://reviews.llvm.org/D34838
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170630/1807a6b0/attachment.html>