[lld] [llvm] [lld][MachO] Follow-up to use madvise() for threaded file page-in. (PR #157917)

[John Holdsworth via llvm-commits]

johnno1962 wrote:

@zygoloid, just an update on what I'm trying with recent commits. I'm seeking to verify the follow change to your change:
```
@@ -506,7 +509,7 @@ getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
       // from the page cache that are not properly filled with trailing zeroes,
       // if some prior user of the page wrote non-zero bytes. Detect this and
       // don't use mmap in that case.
-      if (!RequiresNullTerminator || *Result->getBufferEnd() == '\0')
+      if (!IsText || *Result->getBufferEnd() == '\0')
         return std::move(Result);
     }
   }
```
... along with some other minor changes to the signatures of functions internal to MemoryBuffer.cpp to marshal the IsText value through to getOpenFileImpl(). The difference is IsText has a default argument value of false whereas the default value for RequiresNullTerminator is true when you don't specify a value which seems to be the common case. I appreciate this is a minor fib for the code but there don't seem to be many other solutions given the constraints of a well used entry point. I don't know how many buffers that require null termination are not specified as being text in practice.

Unfortunately CI seems to be broken at the moment so I've not been able to validate the change other than testing on a large actual link my end. I definitely doesn't have the speed regression for lld. I've tried reverting the change and a commit that validated yesterday fails Linux tests so the problem is with the repo.

How would you feel about me making this change? I'll try again tomorrow to get it to pass CI.

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