[PATCH] D42446: [ThinLTO] Add a couple of more knobs to C API to control cache size.

[Katya Romanova via Phabricator via llvm-commits]

kromanova planned changes to this revision.
kromanova added inline comments.


================
Comment at: include/llvm/LTO/legacy/ThinLTOCodeGenerator.h:190
+  /// over the amount of available space on the disk will be reduced to the
+  /// amount of available space. A value of 0 will be ignored.
+  void setCacheMaxSizeBytes(unsigned MaxSizeBytes) {
----------------
kromanova wrote:
> kromanova wrote:
> > tejohnson wrote:
> > > In CachePruning.h, a value of 0 for this field disables size based pruning. Why do something different here (which seems to go in the opposite direction from D42267)?
> > In D42267  a value 0 for PruningInterval actually forces the scan to occur, so its meaning is somewhat different.
> > 
> > 
> > 
> When writing the patch, I was hesitating for a whether to ignore value 0 here (for a time being) or use it for initializing CacheOptions.Policy.* (as a part of this patch).  
> 
> I decided to ignore a value 0 for now and that's why :
> 
> the value 0 for two previously existing pruning control functions is currently ignored (while I believe it should be changed to have the same effect as in C++ AP!)
> - setCacheEntryExpiration
> - setMaxCacheSizeRelativeToAvailableSpace
> 
> I added these two knobs in this patch: 
> - setCacheMaxSizeBytes
> - setCacheMaxSizeFiles
> and decided to keep for now consistent with setCacheEntryExpiration and setMaxCacheSizeRelativeToAvailableSpace, since all of these 4 functions are related.
> 
> I could have changed all of these functions and accept value 0 as a part of this patch, but a lot of tests needs to be written, so I thought it's better to do it as a separate commit.
> 
> 
Also, before making a subsequent commit to accept (instead of ignoring) 0 values for setCache* functions, I'd like to get ld64 developers option that they are OK with this. Who is responsible for ld64 or LTO project in Apple?


https://reviews.llvm.org/D42446