<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 22, 2016 at 11:48 PM, Peter Collingbourne <span dir="ltr"><<a href="mailto:peter@pcc.me.uk" target="_blank">peter@pcc.me.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">What do you think about the below interface changes? It seems like they would address most of the concerns you're raising here.</p>
<p dir="ltr">typedef std::function<AddStreamFn(<wbr>unsigned Task, StringRef Key)>  NativeObjectCache;</p>
<p dir="ltr">NativeObjectCache localCache(std::string CacheDirectoryPath, AddFileFn AddFile);</p>
<p dir="ltr">Error LTO::run(AddStreamFn AddStream, NativeObjectCache Cache) </p>
<p dir="ltr">And remove Cache field from config.</p></blockquote><div><br></div><div>Can't speak for Mehdi, but I do like this change - it makes it much clearer that the AddFile is correlated with the local cache implementation, and doesn't require passing it in to LTO::run when e.g. we are doing regular LTO and no caching. It also seems like it would facilitate supporting an in-memory cache as Mehdi asked, since the NativeObjectCache isn't requring the AddFileFn.</div><div><br></div><div>Teresa</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="HOEnZb"><font color="#888888">
<p dir="ltr">Peter</p></font></span><div class="HOEnZb"><div class="h5">
<div class="gmail_extra"><br><div class="gmail_quote">On Sep 22, 2016 10:25 PM, "Mehdi AMINI" <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mehdi_amini added inline comments.<br>
<br>
================<br>
Comment at: include/llvm/LTO/Config.h:60<br>
@@ +59,3 @@<br>
+/// cache, pass a unique string as the Key. For hits, the cached file will be<br>
+/// added using AddFile and this function will return AddStreamFn(). For misses,<br>
+/// the cache will return a stream callback which must be called at most once to<br>
----------------<br>
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.<br>
<br>
================<br>
Comment at: include/llvm/LTO/Config.h:72<br>
@@ -64,2 +71,3 @@<br>
+    NativeObjectCache;<br>
<br>
 /// LTO configuration. A linker can configure LTO by setting fields in this data<br>
----------------<br>
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?<br>
<br>
================<br>
Comment at: include/llvm/LTO/Config.h:116<br>
@@ -107,1 +115,3 @@<br>
<br>
+  NativeObjectCache Cache;<br>
+<br>
----------------<br>
No documented.<br>
<br>
================<br>
Comment at: include/llvm/LTO/LTO.h:316<br>
@@ +315,3 @@<br>
+  /// AddFile functions to add native object files to the link.<br>
+  ///<br>
+  /// The client will receive at most one callback (via either AddStream or<br>
----------------<br>
> Is there any specific client you have in mind where this would be a problem?<br>
<br>
That's not the angle I'm looking at when looking at the API design.<br>
<br>
For instance:<br>
Why is `NativeObjectCache Cache` in in the config? While these callbacks are here?<br>
<br>
<br>
<br>
<a href="https://reviews.llvm.org/D24622" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2462<wbr>2</a><br>
<br>
<br>
<br>
</blockquote></div></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> 408-460-2413</td></tr></tbody></table></span></div>
</div></div>