Don't use mmap on Windows

[yvvan]

Summary

Memory mapping does not make llvm faster (at least I don't see any difference).
It also allows llvm not to lock files which is sometimes quite important.

[yvvan]

What do you think about that change?

[nik]

Ping.

I guess locking the file less helps also clangd?!

[ilya-biryukov]

This looks strange. AFAIK, memory mapping files in Windows does no 'locking' by itself. I've just written a small program to check that it's possible to modify the file, memory-mapped using Win32 API.
What is specific problem you ran into?

[yvvan]

The files might also be removed.
I checked that with qtcreator from git. Switching to the different branch started reparse of some files.
When I frequently switched branches back and forth I got lock issues. With mmap turned off this did not happen.

[ilya-biryukov]

In D35200#844132, @yvvan wrote:

The files might also be removed.
I checked that with qtcreator from git. Switching to the different branch started reparse of some files.
When I frequently switched branches back and forth I got lock issues. With mmap turned off this did not happen.

Renaming/removing memory mapped files also works fine.

What were those 'lock issues' specifically? MemoryBuffer code that uses the function you're changing seems to be doing the right thing by always allowing shared file access.
We definitely need a test case that realiably reproduces your issue.

[joerg]

The primary reason for using mmap is not so much performance, but reduced memory foot print.

[yvvan]

It was the wrong direction

lib/Support/MemoryBuffer.cpp

 
 static bool shouldUseMmap(int FD,
                           size_t FileSize,
                           size_t MapSize,
                           off_t Offset,
                           bool RequiresNullTerminator,
                           int PageSize,
                           bool IsVolatile) {
+#ifdef _WIN32
+  // Do not use mmap on Windows in order to avoid file locking
+  return false;
+#endif
   // mmap may leave the buffer without null terminator if the file size changed
   // by the time the last page is mapped in, so avoid it if the file size is
   // likely to change.
   if (IsVolatile)
     return false;
 
   // We don't use mmap for small files because this can severely fragment our
   // address space.