<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Oct 5, 2017 at 10:32 AM Peter Collingbourne <<a href="mailto:peter@pcc.me.uk">peter@pcc.me.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 4, 2017 at 7:35 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Ahh, i see what you’re saying.  Dest *becomes* Source for a short time.<br><br>The main thing I’m concerned about with this new implementation is that it increases the complexity quite a bit, and in doing so makes the semantics harder to reason about.  How many additional weird codepaths in the kernel is this tickling now?  So there could be more (or at least different) race conditions.<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>That was a concern for me as well, but it is somewhat mitigated by the fact that it is essentially a reimplementation of ReplaceFile, which is already part of the Windows API, so I'd expect the kernel code paths to be relatively well exercised. From MSDN: "An application can call ReplaceFile instead of calling separate functions to save the data to a new file, rename the original file using a temporary name, rename the new file to have the same name as the original file, and delete the original file." i.e. what my code is doing.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Would it be possible for the ThinLTO processes to map a shared memory region and synchronize on it instead of relying on flaky file system semantics?<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Not easily. The cache needs to be persisted between linker invocations, so I think there would need to be something on the file system. I suppose that you could imagine mmap'ing a large sparse file into every process and synchronizing on that, but there is also a mechanism for pruning old cache entries in order to save disk space (which may run concurrently with another process that is adding entries to the cache), so even if we did have an mmap'ed file, we'd need to arrange for the pruner to release pages to free storage, and to do so without races.</div></div></div></div></blockquote><div><br></div><div>What I mean is that you could have a system-wide named mutex with a unique name that involves a 128-bit GUID or something.  All ThinLTO linker processes could agree on this name from now until forever.  This would presist across linker invocations because the name of the mutex never changes.  This would synchronize access to a single "cache control file" that also had a globally unique (and unchanging) name.  So the algorithm for opening a file from the cache is:</div><div><br></div><div>1) Acquire the mutex</div><div>2) Try to open the file and either succeed or fail</div><div>3) Close the mutex.</div><div><br></div><div>and the algorithm for adding a new file to the cache is:</div><div><br></div><div>1) Acquire the mutex</div><div>2) Do whatever it takes to get a new file into the cache</div><div>3) Close the mutex</div><div><br></div><div>This should never race.</div></div></div>