[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

Mon Dec 3 02:45:11 PST 2018

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

We need to keep mmapping the PCH files.

In D55139#1315944 <https://reviews.llvm.org/D55139#1315944>, @malaperle wrote:

> Hi Ilya. Does this apply to compile_commands.json too? I've seen that problem for that file as well. If not, I understand it can be another patch, just curious.

Yeah, this applies to all files, including `compiler_commands.json`.
I've discussed this with a few smart folks and they pointed out doing it for **all** files is not ok, most problematic are PCH files, which are (1) huge, (2) produced by clangd itself and never modified after being produced. 
(1) means it's very wasteful to fully load them into memory, (2) means it can easily be avoided. That complicated things, though, and the current approach to fix this in libclang D54995 <https://reviews.llvm.org/D54995> (passing an explicit flag when we know the file should be memory-mapped) starts to look more attractive.
I'll patch it up in this patch by looking at file name and avoiding that for PCH files. There are still PCM files, which are very similar, my hopes are they're not widely used on Windows these days so we should be fine for now, but that would definitely bite us in the future.

> Please fix this also for libclang clients. I think it's safe to assume that files might be edited once CXTranslationUnit_PrecompiledPreamble or CXTranslationUnit_CacheCompletionResults is set as flag - that's what clang_defaultEditingTranslationUnitOptions() returns.

libclang clients should be covered by D54995 <https://reviews.llvm.org/D54995> and previous changes around it. As I mentioned in the previous comment, passing a flag when file cannot be memory-mapped seems like a proper solution anyway.

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55139/new/

https://reviews.llvm.org/D55139