Re: [clang-tools-extra] r344513 - [clangd] Minimal implementation of automatic static index (not enabled).

2018-10-16 Thread Sam McCall via cfe-commits
Sorry Douglas. It was indeed a flaky test. Disabled in r344586 and then
fixed in r344595.

On Tue, Oct 16, 2018 at 3:57 AM  wrote:

> Hi Sam,
>
> The test you added in this commit is timing out when run on the PS4
> Windows bot:
>
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20724/steps/test/logs/stdio
>
> command timed out: 1200 seconds without output running ['ninja', '-j',
> '36', 'check-all'], attempting to kill
> FAIL: Extra Tools Unit Tests ::
> clangd/./ClangdTests.exe/BackgroundIndexTest.IndexTwoFiles (44401 of 44401)
>  TEST 'Extra Tools Unit Tests ::
> clangd/./ClangdTests.exe/BackgroundIndexTest.IndexTwoFiles' FAILED
> 
> Note: Google Test filter = BackgroundIndexTest.IndexTwoFiles
>
> [==] Running 1 test from 1 test case.
>
> [--] Global test environment set-up.
>
> [--] 1 test from BackgroundIndexTest
>
> [ RUN  ] BackgroundIndexTest.IndexTwoFiles
>
>
> 
> program finished with exit code 1
> elapsedTime=1446.10
>
> Can you take a look?
>
> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> > Of Sam McCall via cfe-commits
> > Sent: Monday, October 15, 2018 6:34
> > To: cfe-commits@lists.llvm.org
> > Subject: [clang-tools-extra] r344513 - [clangd] Minimal implementation
> > of automatic static index (not enabled).
> >
> > Author: sammccall
> > Date: Mon Oct 15 06:34:10 2018
> > New Revision: 344513
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=344513=rev
> > Log:
> > [clangd] Minimal implementation of automatic static index (not
> > enabled).
> >
> > Summary:
> > See tinyurl.com/clangd-automatic-index for design and goals.
> >
> > Lots of limitations to keep this patch smallish, TODOs everywhere:
> >  - no serialization to disk
> >  - no changes to dynamic index, which now has a much simpler job
> >  - no partitioning of symbols by file to avoid duplication of header
> > symbols
> >  - no reindexing of edited files
> >  - only a single worker thread
> >  - compilation database is slurped synchronously (doesn't scale)
> >  - uses memindex, rebuilds after every file (should be dex,
> > periodically)
> >
> > It's not hooked up to ClangdServer/ClangdLSPServer yet: the layering
> > isn't clear (it should really be in ClangdServer, but ClangdLSPServer
> > has all the CDB interactions).
> >
> > Reviewers: ioeric
> >
> > Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman,
> > kadircet, jfb, cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D53032
> >
> > Added:
> > clang-tools-extra/trunk/clangd/index/Background.cpp
> > clang-tools-extra/trunk/clangd/index/Background.h
> > clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
> > Modified:
> > clang-tools-extra/trunk/clangd/CMakeLists.txt
> > clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
> > clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
> > clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
> >
> > Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> > extra/trunk/clangd/CMakeLists.txt?rev=344513=344512=344513=d
> > iff
> > ===
> > ===
> > --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
> > +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Oct 15 06:34:10
> > 2018
> > @@ -38,6 +38,7 @@ add_clang_library(clangDaemon
> >URI.cpp
> >XRefs.cpp
> >
> > +  index/Background.cpp
> >index/CanonicalIncludes.cpp
> >index/FileIndex.cpp
> >index/Index.cpp
> >
> > Added: clang-tools-extra/trunk/clangd/index/Background.cpp
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> > extra/trunk/clangd/index/Background.cpp?rev=344513=auto
> > ===
> > ===
> > --- clang-tools-extra/trunk/clangd/index/Background.cpp (added)
> > +++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Oct 15
> > 06:34:10 2018
> > @@ -0,0 +1,191 @@
> > +//===-- Background.cpp - Build an index in a background thread ---
> > -===//
> > +//
> > +// The LLVM Compiler Infrastructure
> > +//
> > +// This file is dist

RE: [clang-tools-extra] r344513 - [clangd] Minimal implementation of automatic static index (not enabled).

2018-10-15 Thread via cfe-commits
Hi Sam,

The test you added in this commit is timing out when run on the PS4 Windows bot:

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20724/steps/test/logs/stdio

command timed out: 1200 seconds without output running ['ninja', '-j', '36', 
'check-all'], attempting to kill
FAIL: Extra Tools Unit Tests :: 
clangd/./ClangdTests.exe/BackgroundIndexTest.IndexTwoFiles (44401 of 44401)
 TEST 'Extra Tools Unit Tests :: 
clangd/./ClangdTests.exe/BackgroundIndexTest.IndexTwoFiles' FAILED 

Note: Google Test filter = BackgroundIndexTest.IndexTwoFiles

[==] Running 1 test from 1 test case.

[--] Global test environment set-up.

[--] 1 test from BackgroundIndexTest

[ RUN  ] BackgroundIndexTest.IndexTwoFiles



program finished with exit code 1
elapsedTime=1446.10

Can you take a look?

Douglas Yung

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of Sam McCall via cfe-commits
> Sent: Monday, October 15, 2018 6:34
> To: cfe-commits@lists.llvm.org
> Subject: [clang-tools-extra] r344513 - [clangd] Minimal implementation
> of automatic static index (not enabled).
> 
> Author: sammccall
> Date: Mon Oct 15 06:34:10 2018
> New Revision: 344513
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=344513=rev
> Log:
> [clangd] Minimal implementation of automatic static index (not
> enabled).
> 
> Summary:
> See tinyurl.com/clangd-automatic-index for design and goals.
> 
> Lots of limitations to keep this patch smallish, TODOs everywhere:
>  - no serialization to disk
>  - no changes to dynamic index, which now has a much simpler job
>  - no partitioning of symbols by file to avoid duplication of header
> symbols
>  - no reindexing of edited files
>  - only a single worker thread
>  - compilation database is slurped synchronously (doesn't scale)
>  - uses memindex, rebuilds after every file (should be dex,
> periodically)
> 
> It's not hooked up to ClangdServer/ClangdLSPServer yet: the layering
> isn't clear (it should really be in ClangdServer, but ClangdLSPServer
> has all the CDB interactions).
> 
> Reviewers: ioeric
> 
> Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman,
> kadircet, jfb, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D53032
> 
> Added:
> clang-tools-extra/trunk/clangd/index/Background.cpp
> clang-tools-extra/trunk/clangd/index/Background.h
> clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
> Modified:
> clang-tools-extra/trunk/clangd/CMakeLists.txt
> clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
> clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
> clang-tools-extra/trunk/unittests/clangd/SyncAPI.h
> 
> Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> extra/trunk/clangd/CMakeLists.txt?rev=344513=344512=344513=d
> iff
> ===
> ===
> --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Oct 15 06:34:10
> 2018
> @@ -38,6 +38,7 @@ add_clang_library(clangDaemon
>URI.cpp
>XRefs.cpp
> 
> +  index/Background.cpp
>index/CanonicalIncludes.cpp
>index/FileIndex.cpp
>index/Index.cpp
> 
> Added: clang-tools-extra/trunk/clangd/index/Background.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-
> extra/trunk/clangd/index/Background.cpp?rev=344513=auto
> ===
> ===
> --- clang-tools-extra/trunk/clangd/index/Background.cpp (added)
> +++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Oct 15
> 06:34:10 2018
> @@ -0,0 +1,191 @@
> +//===-- Background.cpp - Build an index in a background thread ---
> -===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===-
> -===//
> +
> +#include "index/Background.h"
> +#include "ClangdUnit.h"
> +#include "Compiler.h"
> +#include "Logger.h"
> +#include "Trace.h"
> +#include "index/IndexAction.h"
> +#include "index/MemIndex.h"
> +#include "index/Serialization.h"
> +#include "llvm/Support/SHA1.h"
> +#include 
> +
> +using namespace llvm;
> +namespace clang {
> +namespace clangd

[clang-tools-extra] r344513 - [clangd] Minimal implementation of automatic static index (not enabled).

2018-10-15 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Mon Oct 15 06:34:10 2018
New Revision: 344513

URL: http://llvm.org/viewvc/llvm-project?rev=344513=rev
Log:
[clangd] Minimal implementation of automatic static index (not enabled).

Summary:
See tinyurl.com/clangd-automatic-index for design and goals.

Lots of limitations to keep this patch smallish, TODOs everywhere:
 - no serialization to disk
 - no changes to dynamic index, which now has a much simpler job
 - no partitioning of symbols by file to avoid duplication of header symbols
 - no reindexing of edited files
 - only a single worker thread
 - compilation database is slurped synchronously (doesn't scale)
 - uses memindex, rebuilds after every file (should be dex, periodically)

It's not hooked up to ClangdServer/ClangdLSPServer yet: the layering
isn't clear (it should really be in ClangdServer, but ClangdLSPServer
has all the CDB interactions).

Reviewers: ioeric

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, jfb, 
cfe-commits

Differential Revision: https://reviews.llvm.org/D53032

Added:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
clang-tools-extra/trunk/unittests/clangd/SyncAPI.h

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=344513=344512=344513=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Oct 15 06:34:10 2018
@@ -38,6 +38,7 @@ add_clang_library(clangDaemon
   URI.cpp
   XRefs.cpp
 
+  index/Background.cpp
   index/CanonicalIncludes.cpp
   index/FileIndex.cpp
   index/Index.cpp

Added: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=344513=auto
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (added)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Oct 15 06:34:10 2018
@@ -0,0 +1,191 @@
+//===-- Background.cpp - Build an index in a background thread 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/Background.h"
+#include "ClangdUnit.h"
+#include "Compiler.h"
+#include "Logger.h"
+#include "Trace.h"
+#include "index/IndexAction.h"
+#include "index/MemIndex.h"
+#include "index/Serialization.h"
+#include "llvm/Support/SHA1.h"
+#include 
+
+using namespace llvm;
+namespace clang {
+namespace clangd {
+
+BackgroundIndex::BackgroundIndex(Context BackgroundContext,
+ StringRef ResourceDir,
+ const FileSystemProvider )
+: SwapIndex(llvm::make_unique()), ResourceDir(ResourceDir),
+  FSProvider(FSProvider), BackgroundContext(std::move(BackgroundContext)),
+  Thread([this] { run(); }) {}
+
+BackgroundIndex::~BackgroundIndex() {
+  stop();
+  Thread.join();
+}
+
+void BackgroundIndex::stop() {
+  {
+std::lock_guard Lock(QueueMu);
+ShouldStop = true;
+  }
+  QueueCV.notify_all();
+}
+
+void BackgroundIndex::run() {
+  WithContext Background(std::move(BackgroundContext));
+  while (true) {
+llvm::Optional Task;
+{
+  std::unique_lock Lock(QueueMu);
+  QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
+  if (ShouldStop) {
+Queue.clear();
+QueueCV.notify_all();
+return;
+  }
+  ++NumActiveTasks;
+  Task = std::move(Queue.front());
+  Queue.pop_front();
+}
+(*Task)();
+{
+  std::unique_lock Lock(QueueMu);
+  assert(NumActiveTasks > 0 && "before decrementing");
+  --NumActiveTasks;
+}
+QueueCV.notify_all();
+  }
+}
+
+void BackgroundIndex::blockUntilIdleForTest() {
+  std::unique_lock Lock(QueueMu);
+  QueueCV.wait(Lock, [&] { return Queue.empty() && NumActiveTasks == 0; });
+}
+
+void BackgroundIndex::enqueue(StringRef Directory,
+  tooling::CompileCommand Cmd) {
+  std::lock_guard Lock(QueueMu);
+  enqueueLocked(std::move(Cmd));
+}
+
+void BackgroundIndex::enqueueAll(StringRef Directory,
+ const tooling::CompilationDatabase ) {
+  trace::Span Tracer("BackgroundIndexEnqueueCDB");
+  // FIXME: this function may be slow. Perhaps enqueue a task to re-read the 
CDB
+  // from disk and enqueue the commands