[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318412: [clangd] Use in-memory preambles in clangd. 
(authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D39843

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdLSPServer.h
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/ClangdUnitStore.cpp
  clang-tools-extra/trunk/clangd/ClangdUnitStore.h
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
  clang-tools-extra/trunk/test/clangd/completion.test
  clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test
  clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -24,6 +24,10 @@
 using namespace clang;
 using namespace clang::clangd;
 
+namespace {
+enum class PCHStorageFlag { Disk, Memory };
+}
+
 static llvm::cl::opt CompileCommandsDir(
 "compile-commands-dir",
 llvm::cl::desc("Specify a path to look for compile_commands.json. If path "
@@ -45,6 +49,15 @@
 PrettyPrint("pretty", llvm::cl::desc("Pretty-print JSON output"),
 llvm::cl::init(false));
 
+static llvm::cl::opt PCHStorage(
+"pch-storage",
+llvm::cl::desc("Storing PCHs in memory increases memory usages, but may "
+   "improve performance"),
+llvm::cl::values(
+clEnumValN(PCHStorageFlag::Disk, "disk", "store PCHs on disk"),
+clEnumValN(PCHStorageFlag::Memory, "memory", "store PCHs in memory")),
+llvm::cl::init(PCHStorageFlag::Disk));
+
 static llvm::cl::opt RunSynchronously(
 "run-synchronously",
 llvm::cl::desc("Parse on main thread. If set, -j is ignored"),
@@ -127,16 +140,27 @@
 CompileCommandsDirPath = CompileCommandsDir;
   }
 
+  bool StorePreamblesInMemory;
+  switch (PCHStorage) {
+  case PCHStorageFlag::Memory:
+StorePreamblesInMemory = true;
+break;
+  case PCHStorageFlag::Disk:
+StorePreamblesInMemory = false;
+break;
+  }
+
   llvm::Optional ResourceDirRef = None;
   if (!ResourceDir.empty())
 ResourceDirRef = ResourceDir;
 
   // Change stdin to binary to not lose \r\n on windows.
   llvm::sys::ChangeStdinToBinary();
 
   // Initialize and run ClangdLSPServer.
-  ClangdLSPServer LSPServer(Out, WorkerThreadsCount, EnableSnippets,
-ResourceDirRef, CompileCommandsDirPath);
+  ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
+EnableSnippets, ResourceDirRef,
+CompileCommandsDirPath);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode;
Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -235,7 +235,7 @@
   // NOTE: we use Buffer.get() when adding remapped files, so we have to make
   // sure it will be released if no error is emitted.
   if (Preamble) {
-Preamble->AddImplicitPreamble(*CI, Buffer.get());
+Preamble->AddImplicitPreamble(*CI, VFS, Buffer.get());
   } else {
 CI->getPreprocessorOpts().addRemappedFile(
 CI->getFrontendOpts().Inputs[0].getFile(), Buffer.get());
@@ -1137,16 +1137,20 @@
 
 std::shared_ptr
 CppFile::Create(PathRef FileName, tooling::CompileCommand Command,
+bool StorePreamblesInMemory,
 std::shared_ptr PCHs,
 clangd::Logger ) {
-  return std::shared_ptr(
-  new CppFile(FileName, std::move(Command), std::move(PCHs), Logger));
+  return std::shared_ptr(new CppFile(FileName, std::move(Command),
+  StorePreamblesInMemory,
+  std::move(PCHs), Logger));
 }
 
 CppFile::CppFile(PathRef FileName, tooling::CompileCommand Command,
+ bool StorePreamblesInMemory,
  std::shared_ptr PCHs,
  clangd::Logger )
-: FileName(FileName), Command(std::move(Command)), RebuildCounter(0),
+: FileName(FileName), Command(std::move(Command)),
+  StorePreamblesInMemory(StorePreamblesInMemory), RebuildCounter(0),
   RebuildInProgress(false), PCHs(std::move(PCHs)), Logger(Logger) {
 
   std::lock_guard Lock(Mutex);
@@ -1290,6 +1294,7 @@
   CppFilePreambleCallbacks SerializedDeclsCollector;
   auto BuiltPreamble = PrecompiledPreamble::Build(
   *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, 

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 123035.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Removed /*ref*/.
- Changed the command-line flag: it's -pch-storage now instead of 
-in-memory-pchs.
- Actually pass the StoreInMemory flag to PrecompiledPreamble::Build.
- Fixed a new usage of ClangdServer after rebase (In ClangdTests).


https://reviews.llvm.org/D39843

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/tool/ClangdMain.cpp
  test/clangd/completion.test
  test/clangd/diagnostics-preamble.test
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -332,6 +332,7 @@
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+/*StorePreamblesInMemory=*/true,
 clangd::CodeCompleteOptions(),
 EmptyLogger::getInstance());
 for (const auto  : ExtraFiles)
@@ -396,6 +397,7 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
   clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
@@ -442,6 +444,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
   clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
@@ -490,7 +493,9 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true,
+  clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
   auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -524,7 +529,9 @@
   "-stdlib=libstdc++"});
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true,
+  clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
   // Just a random gcc version string
@@ -573,7 +580,9 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true,
+  clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
   // No need to sync reparses, because reparses are performed on the calling
   // thread to true.
@@ -641,6 +650,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
   clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
@@ -701,7 +711,8 @@
   clangd::CodeCompleteOptions Opts;
   Opts.Limit = 2;
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  Opts, EmptyLogger::getInstance());
+  /*StorePreamblesInMemory=*/true, Opts,
+  EmptyLogger::getInstance());
 
   auto FooCpp = getVirtualTestFilePath("foo.cpp");
   FS.Files[FooCpp] = "";
@@ -796,7 +807,8 @@
 
   auto TestWithOpts = [&](clangd::CodeCompleteOptions Opts) {
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-Opts, EmptyLogger::getInstance());
+/*StorePreamblesInMemory=*/true, Opts,
+EmptyLogger::getInstance());
 // No need to sync reparses here as there are no asserts on diagnostics (or
 // other async operations).
 Server.addDocument(FooCpp, GlobalCompletion.Text);
@@ -989,6 +1001,7 @@
   {
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 ClangdServer Server(CDB, DiagConsumer, FS, 

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdUnit.cpp:238
   if (Preamble) {
-Preamble->AddImplicitPreamble(*CI, Buffer.get());
+Preamble->AddImplicitPreamble(*CI, /*ref*/ VFS, Buffer.get());
   } else {

sammccall wrote:
> ref is still here :-)
Thx. Missed that. Removed.



Comment at: clangd/tool/ClangdMain.cpp:48
 
+static llvm::cl::opt StorePreamblesInMemory(
+"in-memory-pchs",

sammccall wrote:
> Couple of questions about the flag syntax:
>  - I wonder about the granularity - if we had another temp storage 
> (indexes?), would we want another flag or something shared like 
> `-in-memory-storage`
>  - Having this as a bool seems confusing compared to 
> `-pch-storage={memory/disk}`. (We could also overload such a flag to allow 
> setting the location, but nobody's asked for that yet)
  - Indexes are fundamentally different because we actually want to persist 
them across multiple sessions. But I see your point. I'd go with another flag 
for greater flexibility. We do want clangd work great by-default, so those 
flags are mostly for people who know what they're doing and can figure out 
which flags they want to change and why. Maybe I'm not right, though, and 
people would want those kinds of switches.
  - Totally like the `-pch-storage={memory/disk}`  more, updated the change 
accordingly.




https://reviews.llvm.org/D39843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

As discussed, we should probably bundle together some related options into 
structs to make plumbing easier. Not a blocker here though.

If these options are pure-data, I think it's OK if some layers end up with 
access to other layers' options.




Comment at: clangd/ClangdUnit.cpp:238
   if (Preamble) {
-Preamble->AddImplicitPreamble(*CI, Buffer.get());
+Preamble->AddImplicitPreamble(*CI, /*ref*/ VFS, Buffer.get());
   } else {

ref is still here :-)



Comment at: clangd/tool/ClangdMain.cpp:48
 
+static llvm::cl::opt StorePreamblesInMemory(
+"in-memory-pchs",

Couple of questions about the flag syntax:
 - I wonder about the granularity - if we had another temp storage (indexes?), 
would we want another flag or something shared like `-in-memory-storage`
 - Having this as a bool seems confusing compared to 
`-pch-storage={memory/disk}`. (We could also overload such a flag to allow 
setting the location, but nobody's asked for that yet)


https://reviews.llvm.org/D39843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D39843#920568, @sammccall wrote:

> As a very first step, can we make this configurable but off-by-default? That 
> will address the pressing diskless-servers need.


I think on-disk by default makes sense now until we do the measurements and 
have a the data to prove the on-memory is really better.


https://reviews.llvm.org/D39843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 122845.
ilya-biryukov added a comment.

Made in-memory preambles optional (on-disk by default).


https://reviews.llvm.org/D39843

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/ClangdUnitStore.cpp
  clangd/ClangdUnitStore.h
  clangd/tool/ClangdMain.cpp
  test/clangd/completion.test
  test/clangd/diagnostics-preamble.test
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -332,6 +332,7 @@
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+/*StorePreamblesInMemory=*/true,
 clangd::CodeCompleteOptions(),
 EmptyLogger::getInstance());
 for (const auto  : ExtraFiles)
@@ -396,6 +397,7 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
   clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
@@ -442,6 +444,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
   clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
@@ -490,7 +493,9 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true,
+  clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
   auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -524,7 +529,9 @@
   "-stdlib=libstdc++"});
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true,
+  clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
   // Just a random gcc version string
@@ -573,7 +580,9 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true,
+  clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
   // No need to sync reparses, because reparses are performed on the calling
   // thread to true.
@@ -642,6 +651,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
   clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
@@ -759,7 +769,8 @@
 
   auto TestWithOpts = [&](clangd::CodeCompleteOptions Opts) {
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-Opts, EmptyLogger::getInstance());
+/*StorePreamblesInMemory=*/true, Opts,
+EmptyLogger::getInstance());
 // No need to sync reparses here as there are no asserts on diagnostics (or
 // other async operations).
 Server.addDocument(FooCpp, GlobalCompletion.Text);
@@ -952,6 +963,7 @@
   {
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+/*StorePreamblesInMemory=*/true,
 clangd::CodeCompleteOptions(),
 EmptyLogger::getInstance());
 
@@ -1112,6 +1124,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
   clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
@@ -1238,7 +1251,8 @@
   std::move(StartSecondReparsePromise));
 
   MockCompilationDatabase 

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Heh, I was just going to ask :-)

As a very first step, can we make this configurable but off-by-default? That 
will address the pressing diskless-servers need.

In a typical workstation scenario, disk is plentiful (if slow) and RAM is 
scarce - very likely we're running browser + editor (VSCode is another browser 
really) + clangd + build system, and clangd is likely the least important of 
these...
So if we're going to increase mem usage by a significant amount I think we want 
to measure it carefully first.


https://reviews.llvm.org/D39843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

After looking at the numbers, I think we should make this configurable. I'll 
update this change accordingly. Not sure what should be the default, though. 
I'm currently erring on the side of "in-memory by default".

Current implementation eats almost twice as much memory, which quickly adds up 
(tried comparing with earlier builds on `ClangdUnit.cpp`).
Even without proper benchmarks, it's clear that if we run out of memory (and we 
easily could if run on a machine with, say, 8GB of RAM) on-disk preambles are 
an easy way out of trouble.


https://reviews.llvm.org/D39843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

https://reviews.llvm.org/D39843

Files:
  clangd/ClangdUnit.cpp


Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -235,7 +235,7 @@
   // NOTE: we use Buffer.get() when adding remapped files, so we have to make
   // sure it will be released if no error is emitted.
   if (Preamble) {
-Preamble->AddImplicitPreamble(*CI, Buffer.get());
+Preamble->AddImplicitPreamble(*CI, /*ref*/ VFS, Buffer.get());
   } else {
 CI->getPreprocessorOpts().addRemappedFile(
 CI->getFrontendOpts().Inputs[0].getFile(), Buffer.get());
@@ -1301,7 +1301,7 @@
   CppFilePreambleCallbacks SerializedDeclsCollector;
   auto BuiltPreamble = PrecompiledPreamble::Build(
   *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
-  SerializedDeclsCollector);
+  /*StoreInMemory=*/true, SerializedDeclsCollector);
 
   if (BuiltPreamble) {
 return std::make_shared(


Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -235,7 +235,7 @@
   // NOTE: we use Buffer.get() when adding remapped files, so we have to make
   // sure it will be released if no error is emitted.
   if (Preamble) {
-Preamble->AddImplicitPreamble(*CI, Buffer.get());
+Preamble->AddImplicitPreamble(*CI, /*ref*/ VFS, Buffer.get());
   } else {
 CI->getPreprocessorOpts().addRemappedFile(
 CI->getFrontendOpts().Inputs[0].getFile(), Buffer.get());
@@ -1301,7 +1301,7 @@
   CppFilePreambleCallbacks SerializedDeclsCollector;
   auto BuiltPreamble = PrecompiledPreamble::Build(
   *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
-  SerializedDeclsCollector);
+  /*StoreInMemory=*/true, SerializedDeclsCollector);
 
   if (BuiltPreamble) {
 return std::make_shared(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits