Commit ed98994f authored by Sam McCall's avatar Sam McCall
Browse files

Revert "[clangd] Mechanism to make update debounce responsive to rebuild speed."

This reverts commit 92570718.
Breaking tests: http://45.33.8.238/linux/9296/step_9.txt
parent d4c8230a
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -106,7 +106,7 @@ private:

ClangdServer::Options ClangdServer::optsForTest() {
  ClangdServer::Options Opts;
  Opts.UpdateDebounce = DebouncePolicy::fixed(/*zero*/ {});
  Opts.UpdateDebounce = std::chrono::steady_clock::duration::zero(); // Faster!
  Opts.StorePreamblesInMemory = true;
  Opts.AsyncThreadsCount = 4; // Consistent!
  Opts.SemanticHighlighting = true;
+2 −2
Original line number Diff line number Diff line
@@ -130,8 +130,8 @@ public:
    llvm::Optional<std::string> ResourceDir = llvm::None;

    /// Time to wait after a new file version before computing diagnostics.
    DebouncePolicy UpdateDebounce =
        DebouncePolicy::fixed(std::chrono::milliseconds(500));
    std::chrono::steady_clock::duration UpdateDebounce =
        std::chrono::milliseconds(500);

    bool SuggestMissingIncludes = false;

+7 −53
Original line number Diff line number Diff line
@@ -61,7 +61,6 @@
#include "llvm/Support/Threading.h"
#include <algorithm>
#include <memory>
#include <mutex>
#include <queue>
#include <thread>

@@ -165,7 +164,7 @@ class ASTWorker {
  friend class ASTWorkerHandle;
  ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
            TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync,
            DebouncePolicy UpdateDebounce, bool StorePreamblesInMemory,
            steady_clock::duration UpdateDebounce, bool StorePreamblesInMemory,
            ParsingCallbacks &Callbacks);

public:
@@ -177,7 +176,7 @@ public:
  static ASTWorkerHandle
  create(PathRef FileName, const GlobalCompilationDatabase &CDB,
         TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
         Semaphore &Barrier, DebouncePolicy UpdateDebounce,
         Semaphore &Barrier, steady_clock::duration UpdateDebounce,
         bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
  ~ASTWorker();

@@ -243,7 +242,7 @@ private:
  TUScheduler::ASTCache &IdleASTs;
  const bool RunSync;
  /// Time to wait after an update to see whether another update obsoletes it.
  const DebouncePolicy UpdateDebounce;
  const steady_clock::duration UpdateDebounce;
  /// File that ASTWorker is responsible for.
  const Path FileName;
  const GlobalCompilationDatabase &CDB;
@@ -264,9 +263,6 @@ private:
  /// be consumed by clients of ASTWorker.
  std::shared_ptr<const ParseInputs> FileInputs;         /* GUARDED_BY(Mutex) */
  std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
  /// Times of recent AST rebuilds, used for UpdateDebounce computation.
  llvm::SmallVector<DebouncePolicy::clock::duration, 8>
      RebuildTimes; /* GUARDED_BY(Mutex) */
  /// Becomes ready when the first preamble build finishes.
  Notification PreambleWasBuilt;
  /// Set to true to signal run() to finish processing.
@@ -330,7 +326,7 @@ private:
ASTWorkerHandle
ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB,
                  TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
                  Semaphore &Barrier, DebouncePolicy UpdateDebounce,
                  Semaphore &Barrier, steady_clock::duration UpdateDebounce,
                  bool StorePreamblesInMemory, ParsingCallbacks &Callbacks) {
  std::shared_ptr<ASTWorker> Worker(
      new ASTWorker(FileName, CDB, IdleASTs, Barrier, /*RunSync=*/!Tasks,
@@ -344,7 +340,7 @@ ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB,

ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
                     TUScheduler::ASTCache &LRUCache, Semaphore &Barrier,
                     bool RunSync, DebouncePolicy UpdateDebounce,
                     bool RunSync, steady_clock::duration UpdateDebounce,
                     bool StorePreamblesInMemory, ParsingCallbacks &Callbacks)
    : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
      FileName(FileName), CDB(CDB),
@@ -492,7 +488,6 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {

    // Get the AST for diagnostics.
    llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
    auto RebuildStartTime = DebouncePolicy::clock::now();
    if (!AST) {
      llvm::Optional<ParsedAST> NewAST =
          buildAST(FileName, std::move(Invocation), CompilerInvocationDiags,
@@ -515,19 +510,6 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
    // spam us with updates.
    // Note *AST can still be null if buildAST fails.
    if (*AST) {
      {
        // Try to record the AST-build time, to inform future update debouncing.
        // This is best-effort only: if the lock is held, don't bother.
        auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
        std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock);
        if (Lock.owns_lock()) {
          // Do not let RebuildTimes grow beyond its small-size (i.e. capacity).
          if (RebuildTimes.size() == RebuildTimes.capacity())
            RebuildTimes.erase(RebuildTimes.begin());
          RebuildTimes.push_back(RebuildDuration);
          Mutex.unlock();
        }
      }
      trace::Span Span("Running main AST callback");

      Callbacks.onMainAST(FileName, **AST, RunPublish);
@@ -768,13 +750,13 @@ Deadline ASTWorker::scheduleLocked() {
  assert(!Requests.empty() && "skipped the whole queue");
  // Some updates aren't dead yet, but never end up being used.
  // e.g. the first keystroke is live until obsoleted by the second.
  // We debounce "maybe-unused" writes, sleeping in case they become dead.
  // We debounce "maybe-unused" writes, sleeping 500ms in case they become dead.
  // But don't delay reads (including updates where diagnostics are needed).
  for (const auto &R : Requests)
    if (R.UpdateType == None || R.UpdateType == WantDiagnostics::Yes)
      return Deadline::zero();
  // Front request needs to be debounced, so determine when we're ready.
  Deadline D(Requests.front().AddTime + UpdateDebounce.compute(RebuildTimes));
  Deadline D(Requests.front().AddTime + UpdateDebounce);
  return D;
}

@@ -1054,33 +1036,5 @@ std::vector<Path> TUScheduler::getFilesWithCachedAST() const {
  return Result;
}

DebouncePolicy::clock::duration
DebouncePolicy::compute(llvm::ArrayRef<clock::duration> History) const {
  assert(Min <= Max && "Invalid policy");
  if (History.empty())
    return Max; // Arbitrary.

  // Base the result on the median rebuild.
  // nth_element needs a mutable array, take the chance to bound the data size.
  History = History.take_back(15);
  llvm::SmallVector<clock::duration, 15> Recent(History.begin(), History.end());
  auto Median = Recent.begin() + Recent.size() / 2;
  std::nth_element(Recent.begin(), Median, Recent.end());

  clock::duration Target =
      std::chrono::duration_cast<clock::duration>(RebuildRatio * *Median);
  if (Target > Max)
    return Max;
  if (Target < Min)
    return Min;
  return Target;
}

DebouncePolicy DebouncePolicy::fixed(clock::duration T) {
  DebouncePolicy P;
  P.Min = P.Max = T;
  return P;
}

} // namespace clangd
} // namespace clang
+2 −24
Original line number Diff line number Diff line
@@ -61,28 +61,6 @@ struct ASTRetentionPolicy {
  unsigned MaxRetainedASTs = 3;
};

/// Clangd may wait after an update to see if another one comes along.
/// This is so we rebuild once the user stops typing, not when they start.
/// Debounce may be disabled/interrupted if we must build this version.
/// The debounce time is responsive to user preferences and rebuild time.
/// In the future, we could also consider different types of edits.
struct DebouncePolicy {
  using clock = std::chrono::steady_clock;

  /// The minimum time that we always debounce for.
  clock::duration Min = /*zero*/ {};
  /// The maximum time we may debounce for.
  clock::duration Max = /*zero*/ {};
  /// Target debounce, as a fraction of file rebuild time.
  /// e.g. RebuildRatio = 2, recent builds took 200ms => debounce for 400ms.
  float RebuildRatio = 1;

  /// Compute the time to debounce based on this policy and recent build times.
  clock::duration compute(llvm::ArrayRef<clock::duration> History) const;
  /// A policy that always returns the same duration, useful for tests.
  static DebouncePolicy fixed(clock::duration);
};

struct TUAction {
  enum State {
    Queued,           // The TU is pending in the thread task queue to be built.
@@ -180,7 +158,7 @@ public:

    /// Time to wait after an update to see if another one comes along.
    /// This tries to ensure we rebuild once the user stops typing.
    DebouncePolicy UpdateDebounce;
    std::chrono::steady_clock::duration UpdateDebounce = /*zero*/ {};

    /// Determines when to keep idle ASTs in memory for future use.
    ASTRetentionPolicy RetentionPolicy;
@@ -295,7 +273,7 @@ private:
  // asynchronously.
  llvm::Optional<AsyncTaskRunner> PreambleTasks;
  llvm::Optional<AsyncTaskRunner> WorkerThreads;
  DebouncePolicy UpdateDebounce;
  std::chrono::steady_clock::duration UpdateDebounce;
};

} // namespace clangd
+2 −29
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <algorithm>
#include <chrono>
#include <utility>

namespace clang {
@@ -209,7 +208,7 @@ TEST_F(TUSchedulerTests, Debounce) {
  std::atomic<int> CallbackCount(0);
  {
    auto Opts = optsForTest();
    Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::seconds(1));
    Opts.UpdateDebounce = std::chrono::seconds(1);
    TUScheduler S(CDB, Opts, captureDiags());
    // FIXME: we could probably use timeouts lower than 1 second here.
    auto Path = testPath("foo.cpp");
@@ -362,7 +361,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
  // Run TUScheduler and collect some stats.
  {
    auto Opts = optsForTest();
    Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::milliseconds(50));
    Opts.UpdateDebounce = std::chrono::milliseconds(50);
    TUScheduler S(CDB, Opts, captureDiags());

    std::vector<std::string> Files;
@@ -755,32 +754,6 @@ TEST_F(TUSchedulerTests, CommandLineWarnings) {
  EXPECT_THAT(Diagnostics, IsEmpty());
}

TEST(DebouncePolicy, Compute) {
  namespace c = std::chrono;
  std::vector<DebouncePolicy::clock::duration> History = {
      c::seconds(0),
      c::seconds(5),
      c::seconds(10),
      c::seconds(20),
  };
  DebouncePolicy Policy;
  Policy.Min = c::seconds(3);
  Policy.Max = c::seconds(25);
  // Call Policy.compute(History) and return seconds as a float.
  auto Compute = [&](llvm::ArrayRef<DebouncePolicy::clock::duration> History) {
    using FloatingSeconds = c::duration<float, c::seconds::period>;
    return static_cast<float>(Policy.compute(History) / FloatingSeconds(1));
  };
  EXPECT_NEAR(10, Compute(History), 0.01) << "(upper) median = 10";
  Policy.RebuildRatio = 1.5;
  EXPECT_NEAR(15, Compute(History), 0.01) << "median = 10, ratio = 1.5";
  Policy.RebuildRatio = 3;
  EXPECT_NEAR(25, Compute(History), 0.01) << "constrained by max";
  Policy.RebuildRatio = 0;
  EXPECT_NEAR(3, Compute(History), 0.01) << "constrained by min";
  EXPECT_NEAR(25, Compute({}), 0.01) << "no history -> max";
}

} // namespace
} // namespace clangd
} // namespace clang