[llvm] [Support] Always call FlushFileBuffers() when unmapping memory on Windows (PR #78597)

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 18 12:17:24 PST 2024


mstorsjo wrote:

> > > @mstorsjo Are you able to test linking a large PDB by chance? 4GB+ PDB are becoming quite common nowadays, and since the PDB isn’t needed just after link, the previous optimization I did was saving several seconds of link time.
> > 
> > 
> > I don't have any such scenarios available to test off the bat at least. To understand - what bit is it that takes the extra time in that case? Does `FlushFileBuffers()` force flushing the large PDB to actual disk, while it otherwise only would keep it in disk cache in memory (and maybe not flush it to disk at all in the end, if it doesn't end up used/needed)?
> 
> `FlushFileBuffers()` does indeed wait until all data is physically written on the storage medium. If I remember correctly the Windows driver for that medium also asks the ROM on the storage device to flush its internal buffers as well, which it doesn't normally do, so this can be slow even with SSDs.
> 
> > What would be the right way to test this? We produce PDB files bigger than 4GB so it's not a huge deal for me to test.
> 
> Try `lld-link .. /time` before/after patch? Or `Measure-Command { lld-link .. }` with powershell if you want to be more precise.

If you can do that, and also measure the performance impact of this change (on top of this one), that'd be great:
```diff
diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc
index 204dc412972d..9c75fafc5fdf 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -938,6 +938,7 @@ mapped_file_region::mapped_file_region(sys::fs::file_t fd, mapmode mode,
 
 void mapped_file_region::unmapImpl() {
   if (Mapping) {
+    ::FlushViewOfFile(Mapping, Size);
     ::UnmapViewOfFile(Mapping);
 
     if (Mode == mapmode::readwrite) {
@@ -953,7 +954,7 @@ void mapped_file_region::unmapImpl() {
       // always end up unflushed (regardless of version of Windows, unless flushed
       // with this explicit call, if they are renamed with
       // SetFileInformationByHandle(FileRenameInfo) before closing the output handle.
-      ::FlushFileBuffers(FileHandle);
+//      ::FlushFileBuffers(FileHandle);
     }
 
     ::CloseHandle(FileHandle);
```

https://github.com/llvm/llvm-project/pull/78597


More information about the llvm-commits mailing list