Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[clangd] Don't mmap source files on all platforms --> don't crash on …
…git checkout Summary: Previously we mmapped on unix and not on windows: on windows mmap takes an exclusive lock on the file and prevents the user saving changes! The failure mode on linux is a bit more subtle: if the file is changed on disk but the SourceManager sticks around, then subsequent operations on the SourceManager will fail as invariants are violated (e.g. null-termination). This commonly manifests as crashes after switching git branches with many files open in clangd. Nominally mmap is for performance here, and we should be willing to give some up to stop crashing. Measurements on my system (linux+desktop+SSD) at least show no measurable regression on an a fairly IO-heavy workload: drop disk caches, open SemaOverload.cpp, wait for first diagnostics. for i in `seq 100`; do for variant in mmap volatile; do echo 3 | sudo tee /proc/sys/vm/drop_caches /usr/bin/time --append --quiet -o ~/timings -f "%C %E" \ bin/clangd.$variant -sync -background-index=0 < /tmp/mirror > /dev/null done done bin/clangd.mmap -sync -background-index=0 0:07.60 bin/clangd.volatile -sync -background-index=0 0:07.89 bin/clangd.mmap -sync -background-index=0 0:07.44 bin/clangd.volatile -sync -background-index=0 0:07.89 bin/clangd.mmap -sync -background-index=0 0:07.42 bin/clangd.volatile -sync -background-index=0 0:07.50 bin/clangd.mmap -sync -background-index=0 0:07.90 bin/clangd.volatile -sync -background-index=0 0:07.53 bin/clangd.mmap -sync -background-index=0 0:07.64 bin/clangd.volatile -sync -background-index=0 0:07.55 bin/clangd.mmap -sync -background-index=0 0:07.75 bin/clangd.volatile -sync -background-index=0 0:07.47 bin/clangd.mmap -sync -background-index=0 0:07.90 bin/clangd.volatile -sync -background-index=0 0:07.50 bin/clangd.mmap -sync -background-index=0 0:07.81 bin/clangd.volatile -sync -background-index=0 0:07.95 bin/clangd.mmap -sync -background-index=0 0:07.55 bin/clangd.volatile -sync -background-index=0 0:07.65 bin/clangd.mmap -sync -background-index=0 0:08.15 bin/clangd.volatile -sync -background-index=0 0:07.54 bin/clangd.mmap -sync -background-index=0 0:07.78 bin/clangd.volatile -sync -background-index=0 0:07.61 bin/clangd.mmap -sync -background-index=0 0:07.78 bin/clangd.volatile -sync -background-index=0 0:07.55 bin/clangd.mmap -sync -background-index=0 0:07.41 bin/clangd.volatile -sync -background-index=0 0:07.40 bin/clangd.mmap -sync -background-index=0 0:07.54 bin/clangd.volatile -sync -background-index=0 0:07.42 bin/clangd.mmap -sync -background-index=0 0:07.45 bin/clangd.volatile -sync -background-index=0 0:07.49 bin/clangd.mmap -sync -background-index=0 0:07.95 bin/clangd.volatile -sync -background-index=0 0:07.66 bin/clangd.mmap -sync -background-index=0 0:08.04 Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73617 (cherry picked from commit b500c49)
- Loading branch information