<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p style="margin-top:0;margin-bottom:0">Hi!</p>
<p style="margin-top:0;margin-bottom:0">Yes, we also still have that issue on Windows and have some of our own bug reports (<a href="https://bugreports.qt.io/browse/QTCREATORBUG-15449" class="OWAAutoLink" id="LPlnk144555" previewremoved="true">https://bugreports.qt.io/browse/QTCREATORBUG-15449</a>)</p>
<p style="margin-top:0;margin-bottom:0">For some time we used the workaround from <span>D35200 but abandoned it finally because of the complaints about high memory consumption.</span></p>
<p style="margin-top:0;margin-bottom:0"><span><br>
</span></p>
<p style="margin-top:0;margin-bottom:0"><span>There's no way I know to fix it because AFAIK windows always locks mmapped files. But i can't say that this behavior is broken because you are not supposed to mark file 'system' if you plan to modify it. What would
I do if I had bugreport with '<span style="color: rgb(33, 33, 33); font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif, serif, EmojiFont; font-size: 15px;">#pragma system_header' - I would simply remove that line from the unsaved file
content I pass to clang.</span></span></p>
<p style="margin-top:0;margin-bottom:0"><span><span style="color: rgb(33, 33, 33); font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif, serif, EmojiFont; font-size: 15px;"><br>
</span></span></p>
<p style="margin-top:0;margin-bottom:0"><span><span style="color: rgb(33, 33, 33); font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif, serif, EmojiFont; font-size: 15px;">Regards,</span></span></p>
<p style="margin-top:0;margin-bottom:0"><span><span style="color: rgb(33, 33, 33); font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif, serif, EmojiFont; font-size: 15px;">Ivan</span></span></p>
<br>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Ilya Biryukov <ibiryukov@google.com><br>
<b>Sent:</b> Tuesday, July 24, 2018 9:13:47 AM<br>
<b>To:</b> Dmitry.Kozhevnikov@jetbrains.com; Ivan Donchevskii<br>
<b>Cc:</b> via cfe-dev<br>
<b>Subject:</b> Re: [cfe-dev] Locking files on Windows with clangd</font>
<div> </div>
</div>
<meta content="text/html; charset=utf-8">
<div>
<div dir="ltr">Hi Dmitry,
<div><br>
</div>
<div><span style="font-size:small; background-color:rgb(255,255,255); float:none; display:inline">We've had a pull request with a similar fix at some point: <a href="https://reviews.llvm.org/D35200">https://reviews.llvm.org/D35200</a>, but</span> it was abandoned
and never made it in.</div>
<div><a class="x_gmail_plusreply" id="x_gmail-plusReplyChip-3" href="mailto:ivan.donchevskii@qt.io" tabindex="-1">@ivan.donchevskii@qt.io</a>, did you manage to figure out a way to make this work on Windows? Any ideas on what's the best way to fix this?<br>
</div>
<div><br>
</div>
</div>
<br>
<div class="x_gmail_quote">
<div dir="ltr">On Sun, Jul 22, 2018 at 7:22 PM <a href="mailto:Dmitry.Kozhevnikov@jetbrains.com">
Dmitry.Kozhevnikov@jetbrains.com</a> via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex">
<div style="word-wrap:break-word; line-break:after-white-space">Hi!<br>
<br>
A short background: we at JetBrains are experimenting with using clangd with<br>
CLion, and we're very happy with the results so far. Unfortunately, we've hit<br>
a problem when header files are locked on Windows, so they can't be saved,<br>
they break `git rebase` (leading to the loss of work), etc.<br>
<br>
It happens when the file is opened using a memory mapped file. It's fairly<br>
easy to reproduce with clangd:<br>
<br>
1. Create a header big enough to pass the threshold in `shouldUseMmap()`<br>
(lib/Support/MemoryBuffer.cpp)<br>
2. Include it in a source file<br>
3. Keep this source file opened via clangd<br>
4. Now an attempt to modify this header will fail<br>
<br>
The situation is especially unpleasant because the handle is locked not only<br>
during the parse, but for all the time clangd holds the preamble containing<br>
this header.<br>
<br>
This issue is mitigated to an extent in libclang: non-system files are<br>
considered "volatile" (see r160074, r334070), so memory mapped files are not<br>
used for them. However, I feel like it's not enough: you can easily have a<br>
`#pragma system_header` in your codebase (IIUC it affects the mentioned<br>
behavior), locking such file and i.e. losing user's work during `git rebase`<br>
is still unacceptable.<br>
<br>
Also, I think the fact that the proper compiler has this behavior is also<br>
unfortunate (forgotten build in the background might lead to the loss of data).<br>
<br>
IIUC the main reason for having memory mapped files is to reduce the memory<br>
footprint. However, for a regular translation unit, headers usually take several<br>
megabytes, which IMO is tolerable, and is usually topped by other per-TU data<br>
anyway.<br>
<br>
Hence, what do you think about not using memory mapped files on Windows at all<br>
for source files? Are there any implications that I'm not aware of?<br>
<br>
The naive patch (obviously not for commit, just for the illustration):<br>
<br>
--- a/lib/Basic/SourceManager.cpp<br>
+++ b/lib/Basic/SourceManager.cpp<br>
@@ -109,7 +109,12 @@ llvm::MemoryBuffer *ContentCache::getBuffer(DiagnosticsEngine &Diag,<br>
return Buffer.getPointer();<br>
}<br>
<br>
+#ifndef _WIN32<br>
bool isVolatile = SM.userFilesAreVolatile() && !IsSystemFile;<br>
+#else<br>
+ bool isVolatile = true;<br>
+#endif<br>
+<br>
auto BufferOrError =<br>
SM.getFileManager().getBufferForFile(ContentsEntry, isVolatile);<br>
<br>
If I've not missed something, this would effectively disable memory mapped<br>
files for source files on Windows, but they would still be used for<br>
other potentially large files (including PCHs and preambles).<br>
<br>
If this is unacceptable, at the very least, clangd should treat user files<br>
similar to libclang (we can work on a patch in this case).<br>
<br>
Note that there is a similar bug report for QtCreator/libclang with some<br>
interesting comments: <a href="https://bugreports.qt.io/browse/QTCREATORBUG-15449" target="_blank">https://bugreports.qt.io/browse/QTCREATORBUG-15449</a><br>
<br>
Cheers,<br>
Dmitry.</div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="x_gmail_signature">
<div dir="ltr">
<div>
<div dir="ltr">
<div>Regards,</div>
<div>Ilya Biryukov</div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>