On 08/11/2013 20:34, Daniel Jasper wrote: > The patch itself looks good (I don't know why we didn't do that in the > first place). > > > On Fri, Nov 8, 2013 at 12:07 AM, Alp Toker <[email protected] > <mailto:[email protected]>> wrote: > > Author: alp > Date: Fri Nov 8 02:07:19 2013 > New Revision: 194250 > > URL: http://llvm.org/viewvc/llvm-project?rev=194250&view=rev > Log: > clang-format: Write files atomically > > Switch clang-format over to Rewriter::overwriteChangedFiles(). > > The previous implementation was attempting to stream back directly > to the > original file and failing if it was already memory mapped by > MemoryBuffer, > an operation unsupported by Windows. > > MemoryBuffer generally mmaps files larger than the physical page > size so > this will have been difficult to reproduce consistently. > > This change also reduces flicker in code editors and IDEs on all > platforms > when reformatting in-place. > > > I think it is generally a bad idea for IDEs to use in-place > reformatting, but ok ..
This was referring to running clang-format on the commandline while you have files open in XCode. From time to time it went wrong, especially with unsaved changes. > > Note that other incorrect uses of MemoryBuffer exist in LLVM/clang and > will need a similar fix. > > A test should be added for Windows when libFormat performance > issues are > fixed (it takes longer than a day to format a 1MB file at present!) > > > Is it the write that takes so long (i.e. fixed with this patch)? > > If not, can the file be shared (or changed enough so that it can be > shared)? This happened with various internal sources pasted together, but I think it can also be reproduced with sqlite.c: http://www.sqlite.org/2013/sqlite-amalgamation-3080100.zip (My RewriteBuffer patch doesn't help with this one.) For now I'm continuing to track down the MemoryBuffer issues in other parts of LLVM making the same mistake as clang-format.cpp. So won't be looking at this performance issue for a while -- feel free to pick it up.. Alp. > > Modified: > cfe/trunk/tools/clang-format/ClangFormat.cpp > > Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=194250&r1=194249&r2=194250&view=diff > > ============================================================================== > --- cfe/trunk/tools/clang-format/ClangFormat.cpp (original) > +++ cfe/trunk/tools/clang-format/ClangFormat.cpp Fri Nov 8 > 02:07:19 2013 > @@ -213,18 +213,8 @@ static bool format(std::string FileName) > Rewriter Rewrite(Sources, LangOptions()); > tooling::applyAllReplacements(Replaces, Rewrite); > if (Inplace) { > - if (Replaces.size() == 0) > - return false; // Nothing changed, don't touch the file. > - > - std::string ErrorInfo; > - llvm::raw_fd_ostream FileStream(FileName.c_str(), ErrorInfo, > - llvm::sys::fs::F_Binary); > - if (!ErrorInfo.empty()) { > - llvm::errs() << "Error while writing file: " << ErrorInfo > << "\n"; > + if (Rewrite.overwriteChangedFiles()) > return true; > - } > - Rewrite.getEditBuffer(ID).write(FileStream); > - FileStream.flush(); > } else { > if (Cursor.getNumOccurrences() != 0) > outs() << "{ \"Cursor\": " << tooling::shiftedCodePosition( > > > _______________________________________________ > cfe-commits mailing list > [email protected] <mailto:[email protected]> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > -- http://www.nuanti.com the browser experts _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
