Don't instantiate a full host toolchain in ASTMatchersTest.

[jlebar]

Summary

This test was stat()'ing large swaths of /usr/lib hundreds of times, as
every invocation of matchesConditionally*() created a new Linux
toolchain.

In addition to being slow, perf indicated this was causing substantial
contention in the kernel.

Something is...interesting in the kernel, as without this patch I
sometimes see ~11m spent in the kernel, and sometimes ~5m. This
corresponds to bimodal ninja check-clang times of ~30s and ~20s.

It's not clear to me exactly what causes the bimodality. In any case,
this change makes this test run in 2.5s, down from 17s, and it seems to
cause us to get the 20s ninja check-clang time unconditionally.

[klimek]

unittests/ASTMatchers/ASTMatchersTest.h

77 ↗

(On Diff #62123)

typo: instantiting

81–83 ↗

(On Diff #62123)

Couldn't we use -ccc-install-dir to prevent toolchain search?

[jlebar]

unittests/ASTMatchers/ASTMatchersTest.h

81–83 ↗

(On Diff #62123)

Couldn't we use -ccc-install-dir to prevent toolchain search?

I tried -ccc-install-dir /does/not/exist and it had no speedup relative to the original.

[chandlerc]

unittests/ASTMatchers/ASTMatchersTest.h

81–83 ↗

(On Diff #62123)

Yea, that just won't help sadly.

As I indicated to Justin, I think the correct fix long-term is to expose a routine that uses CC1 flags directly for use in unittests. That will completely eliminate the need for this. Fundamentally, the unittests shouldn't be reaching through the entire driver, they should be directly running the frontend. There should be specific and dedicated unittests for the driver bits that point the driver at a fake tree instead of the real tree as well.

But I think this is a reasonable workaround until such an API can be provided.

[jlebar]

Fix typo in comment.

[jlebar]

But I think this is a reasonable workaround until such an API can be provided.

Should I take that as an LG, or are we waiting for someone else to approve this?

[chandlerc]

In D21810#471486, @jlebar wrote:

But I think this is a reasonable workaround until such an API can be provided.

Should I take that as an LG, or are we waiting for someone else to approve this?

Lacking a more detailed response from Manuel, yea, I think you should check this in... I wrote most of the toolchain search logic you're turning off so I'm happy to provide an LGTM here. I think this is fine for now, fixes an immediate issue, and can be revisited and improved later easily. So yes, LG.

cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.h

testing::AssertionResult matchesConditionally(
   MatchFinder Finder;
   VerifyMatch VerifyFound(nullptr, &Found);
   Finder.addMatcher(AMatcher, &VerifyFound);
   VerifyMatch VerifyDynamicFound(nullptr, &DynamicFound);
   if (!Finder.addDynamicMatcher(AMatcher, &VerifyDynamicFound))
     return testing::AssertionFailure() << "Could not add dynamic matcher";
   std::unique_ptr<FrontendActionFactory> Factory(
       newFrontendActionFactory(&Finder));
-  // Some tests use typeof, which is a gnu extension.
-  std::vector<std::string> Args;
-  Args.push_back(CompileArg);
-  // Some tests need rtti/exceptions on
-  Args.push_back("-frtti");
-  Args.push_back("-fexceptions");
+  // Some tests need rtti/exceptions on.  Use an unknown-unknown triple so we
+  // don't instantiate the full system toolchain.  On Linux, instantiating the
+  // toolchain involves stat'ing large portions of /usr/lib, and this slows down
+  // not only this test, but all other tests, via contention in the kernel.
+  //
+  // FIXME: This is a hack to work around the fact that there's no way to do the
+  // equivalent of runToolOnCodeWithArgs without instantiating a full Driver.
+  // We should consider having a function, at least for tests, that invokes cc1.
+  std::vector<std::string> Args = {CompileArg, "-frtti", "-fexceptions",
+                                   "-target", "i386-unknown-unknown"};
   if (!runToolOnCodeWithArgs(
           Factory->create(), Code, Args, Filename, "clang-tool",
           std::make_shared<PCHContainerOperations>(), VirtualMappedFiles)) {
     return testing::AssertionFailure() << "Parsing error in \"" << Code << "\"";
   }
   if (Found != DynamicFound) {
     return testing::AssertionFailure() << "Dynamic match result ("
                                        << DynamicFound
testing::AssertionResult matchesConditionallyWithCuda(
   MatchFinder Finder;
   VerifyMatch VerifyFound(nullptr, &Found);
   Finder.addMatcher(AMatcher, &VerifyFound);
   VerifyMatch VerifyDynamicFound(nullptr, &DynamicFound);
   if (!Finder.addDynamicMatcher(AMatcher, &VerifyDynamicFound))
     return testing::AssertionFailure() << "Could not add dynamic matcher";
   std::unique_ptr<FrontendActionFactory> Factory(
       newFrontendActionFactory(&Finder));
-  // Some tests use typeof, which is a gnu extension.
-  std::vector<std::string> Args;
-  Args.push_back("-xcuda");
-  Args.push_back("-fno-ms-extensions");
-  Args.push_back("--cuda-host-only");
-  Args.push_back("-nocudainc");
-  Args.push_back(CompileArg);
+  // Some tests use typeof, which is a gnu extension.  Using an explicit
+  // unknown-unknown triple is good for a large speedup, because it lets us
+  // avoid constructing a full system triple.
+  std::vector<std::string> Args = {
+      "-xcuda",  "-fno-ms-extensions",      "--cuda-host-only", "-nocudainc",
+      "-target", "nvptx64-unknown-unknown", CompileArg};
   if (!runToolOnCodeWithArgs(Factory->create(),
                              CudaHeader + Code, Args)) {
     return testing::AssertionFailure() << "Parsing error in \"" << Code << "\"";
   }
   if (Found != DynamicFound) {
     return testing::AssertionFailure() << "Dynamic match result ("
                                        << DynamicFound
                                        << ") does not match static result ("
matchAndVerifyResultConditionally(const std::string &Code, const T &AMatcher,
                                   std::unique_ptr<BoundNodesCallback> FindResultVerifier,
                                   bool ExpectResult) {
   bool VerifiedResult = false;
   MatchFinder Finder;
   VerifyMatch VerifyVerifiedResult(std::move(FindResultVerifier), &VerifiedResult);
   Finder.addMatcher(AMatcher, &VerifyVerifiedResult);
   std::unique_ptr<FrontendActionFactory> Factory(
       newFrontendActionFactory(&Finder));
-  // Some tests use typeof, which is a gnu extension.
-  std::vector<std::string> Args(1, "-std=gnu++98");
+  // Some tests use typeof, which is a gnu extension.  Using an explicit
+  // unknown-unknown triple is good for a large speedup, because it lets us
+  // avoid constructing a full system triple.
+  std::vector<std::string> Args = {"-std=gnu++98", "-target",
+                                   "i386-unknown-unknown"};
   if (!runToolOnCodeWithArgs(Factory->create(), Code, Args)) {
     return testing::AssertionFailure() << "Parsing error in \"" << Code << "\"";
   }
   if (!VerifiedResult && ExpectResult) {
     return testing::AssertionFailure()
       << "Could not verify result in \"" << Code << "\"";
   } else if (VerifiedResult && !ExpectResult) {
     return testing::AssertionFailure()