[PATCH] D24622: LTO: Simplify caching interface.

[Peter Collingbourne via llvm-commits]

What do you think about the below interface changes? It seems like they
would address most of the concerns you're raising here.

typedef std::function<AddStreamFn(unsigned Task, StringRef Key)>
NativeObjectCache;

NativeObjectCache localCache(std::string CacheDirectoryPath, AddFileFn
AddFile);

Error LTO::run(AddStreamFn AddStream, NativeObjectCache Cache)

And remove Cache field from config.

Peter

On Sep 22, 2016 10:25 PM, "Mehdi AMINI" <mehdi.amini at apple.com> wrote:

> mehdi_amini added inline comments.
>
> ================
> Comment at: include/llvm/LTO/Config.h:60
> @@ +59,3 @@
> +/// cache, pass a unique string as the Key. For hits, the cached file
> will be
> +/// added using AddFile and this function will return AddStreamFn(). For
> misses,
> +/// the cache will return a stream callback which must be called at most
> once to
> ----------------
> The "AddFile" part seems convoluted to me. It seems to be here only to
> satisfy the flow of having a stream as a returned type.
>
> ================
> Comment at: include/llvm/LTO/Config.h:72
> @@ -64,2 +71,3 @@
> +    NativeObjectCache;
>
>  /// LTO configuration. A linker can configure LTO by setting fields in
> this data
> ----------------
> Since you're claiming to make it easier to change the cache implementation
> in the description, can you clarify how would I implement a "in-memory"
> cache with this interface?
>
> ================
> Comment at: include/llvm/LTO/Config.h:116
> @@ -107,1 +115,3 @@
>
> +  NativeObjectCache Cache;
> +
> ----------------
> No documented.
>
> ================
> Comment at: include/llvm/LTO/LTO.h:316
> @@ +315,3 @@
> +  /// AddFile functions to add native object files to the link.
> +  ///
> +  /// The client will receive at most one callback (via either AddStream
> or
> ----------------
> > Is there any specific client you have in mind where this would be a
> problem?
>
> That's not the angle I'm looking at when looking at the API design.
>
> For instance:
> Why is `NativeObjectCache Cache` in in the config? While these callbacks
> are here?
>
>
>
> https://reviews.llvm.org/D24622
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160922/9da36596/attachment.html>