[llvm] [Support] Avoid a VirtualBox shared folders mmap bug (PR #78597)

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 22 23:49:04 PST 2024


mstorsjo wrote:

> > > I updated the patch to call `is_local`, and do a flush only if that indicates that the file system isn't local, unless we're flushing to avoid the kernel bug anyway.
> > > For the rare kernel bug case, we don't know if calling `FlushViewOfFile` instead of `FlushFileBuffers` works. (It probably does, but verifying it is nontrivial.) Therefore, we probably don't want to change that case. For the new heuristic case, I kept calling `FlushFileBuffers` just for code simplicity reasons, to allow using the same codepath for both cases, because the performance difference between the two flush alternatives seems to be very small in practice.
> > > This keeps the usual much faster performance for writing large PDB files on normal storage, while fixing the correctness in the VirtualBox case.
> > 
> > 
> > @aganea Does this seem like a reasonable compromise, performance wise?
> 
> Yes, thank you for doing this! I would assume the `is_local` check shouldn't affect the current timings?

It shouldn't affect the timings notably, no.

In https://reviews.llvm.org/D155579 we concluded that `is_local`, and in particular, `GetVolumePathNameW`, is slow, but that was from the point of view of essentially `stat()`ing thousands of files. From the point of view of the linker, where we close 1-2 files that we've written, the effect of this is well below the level of noise in the benchmark numbers anyway.

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


More information about the llvm-commits mailing list