[PATCH] D51996: [clangd] Simplify cancellation public API
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE342130: [clangd] Simplify cancellation public API
(authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51996?vs=165158=165251#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51996
Files:
clangd/Cancellation.cpp
clangd/Cancellation.h
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/CancellationTests.cpp
Index: unittests/clangd/CancellationTests.cpp
===
--- unittests/clangd/CancellationTests.cpp
+++ unittests/clangd/CancellationTests.cpp
@@ -13,57 +13,48 @@
namespace {
TEST(CancellationTest, CancellationTest) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
+ auto Task = cancelableTask();
+ WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
- TH->cancel();
+ Task.second();
EXPECT_TRUE(isCancelled());
}
-TEST(CancellationTest, TaskTestHandleDiesContextLives) {
+TEST(CancellationTest, CancelerDiesContextLives) {
llvm::Optional ContextWithCancellation;
{
-TaskHandle TH = Task::createHandle();
-ContextWithCancellation.emplace(setCurrentTask(TH));
+auto Task = cancelableTask();
+ContextWithCancellation.emplace(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
EXPECT_TRUE(isCancelled());
}
TEST(CancellationTest, TaskContextDiesHandleLives) {
- TaskHandle TH = Task::createHandle();
+ auto Task = cancelableTask();
{
-WithContext ContextWithCancellation(setCurrentTask(TH));
+WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
// Still should be able to cancel without any problems.
- TH->cancel();
-}
-
-TEST(CancellationTest, CancellationToken) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
- const auto = getCurrentTask();
- EXPECT_FALSE(CT.isCancelled());
- TH->cancel();
- EXPECT_TRUE(CT.isCancelled());
+ Task.second();
}
TEST(CancellationTest, AsynCancellationTest) {
std::atomic HasCancelled(false);
Notification Cancelled;
- auto TaskToBeCancelled = [&](ConstTaskHandle CT) {
-WithContext ContextGuard(setCurrentTask(std::move(CT)));
+ auto TaskToBeCancelled = [&](Context Ctx) {
+WithContext ContextGuard(std::move(Ctx));
Cancelled.wait();
HasCancelled = isCancelled();
};
- TaskHandle TH = Task::createHandle();
- std::thread AsyncTask(TaskToBeCancelled, TH);
- TH->cancel();
+ auto Task = cancelableTask();
+ std::thread AsyncTask(TaskToBeCancelled, std::move(Task.first));
+ Task.second();
Cancelled.notify();
AsyncTask.join();
Index: clangd/Cancellation.h
===
--- clangd/Cancellation.h
+++ clangd/Cancellation.h
@@ -6,124 +6,82 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-// Cancellation mechanism for async tasks. Roughly all the clients of this code
-// can be classified into three categories:
-// 1. The code that creates and schedules async tasks, e.g. TUScheduler.
-// 2. The callers of the async method that can cancel some of the running tasks,
-// e.g. `ClangdLSPServer`
-// 3. The code running inside the async task itself, i.e. code completion or
-// find definition implementation that run clang, etc.
-//
-// For (1), the guideline is to accept a callback for the result of async
-// operation and return a `TaskHandle` to allow cancelling the request.
-//
-// TaskHandle someAsyncMethod(Runnable T,
-// function)> Callback) {
-// auto TH = Task::createHandle();
-// WithContext ContextWithCancellationToken(TH);
-// auto run = [](){
-// Callback(T());
+// Cancellation mechanism for long-running tasks.
+//
+// This manages interactions between:
+//
+// 1. Client code that starts some long-running work, and maybe cancels later.
+//
+// std::pair Task = cancelableTask();
+// {
+// WithContext Cancelable(std::move(Task.first));
+// Expected
+// deepThoughtAsync([](int answer){ errs() << answer; });
// }
-// // Start run() in a new async thread, and make sure to propagate Context.
-// return TH;
-// }
-//
-// The callers of async methods (2) can issue cancellations and should be
-// prepared to handle `TaskCancelledError` result:
-//
-// void Caller() {
-// // You should store this handle if you wanna cancel the task later on.
-// TaskHandle TH = someAsyncMethod(Task, [](llvm::Expected R) {
-// if(/*check for task cancellation error*/)
-// // Handle the error
-// // Do
[PATCH] D51996: [clangd] Simplify cancellation public API
sammccall added a comment. In https://reviews.llvm.org/D51996#1232770, @ilya-biryukov wrote: > > but the thing you're spending the CPU on is checking for cancellation... > > Unless checking for cancellation is really cheap (which is doable, I think). > We should probably hit some of those in practice before doing something, > though. My point is that if a few pointer traversals/comparisons is significant, then the loop is so fast that checking for cancellations doesn't make sense anyway - we're aiming to save 10s of ms, not a few hundred nanos. Like you say, maybe there are important cases where we can't tell that we're calling frequently, or can't easily avoid that. We don't have any yet though, so optimizing this seems premature. Comment at: clangd/Cancellation.h:81 +/// Always false if there is no active cancelable task. +/// This isn't free (context lookup) - don't call it in a tight loop. +bool isCancelled(); ilya-biryukov wrote: > Allowing fast checking for cancellation seem important. > E.g. if we want to cancel in the middle of parsing, we'll probably be faced > with sema callbacks, frequency of which we don't control, so we'd better > design something that's fast to check. > > I see two ways to deal with this: > - Keep the current design, but cache last access to context key. > - Add an API to get something that can quickly check for cancellation, i.e. > something that would hold a reference to resolved context key. > > Both could be added later, though, and I don't have any data, it's just my > intuition. Yeah, we have a few options if we get there here. Maybe the simplest is ``` if (LLVM_UNLIKELY(++Counter % 1000 == 0)) if (isCancelled()) return; ``` (obviously this is limited, but it depends what problem we're facing) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51996: [clangd] Simplify cancellation public API
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51996: [clangd] Simplify cancellation public API
ilya-biryukov added a comment. Will let Kadir finish the review, consider lgtm'ed from me ;-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51996: [clangd] Simplify cancellation public API
ilya-biryukov added a comment. > but the thing you're spending the CPU on is checking for cancellation... Unless checking for cancellation is really cheap (which is doable, I think). We should probably hit some of those in practice before doing something, though. Comment at: clangd/Cancellation.h:81 +/// Always false if there is no active cancelable task. +/// This isn't free (context lookup) - don't call it in a tight loop. +bool isCancelled(); Allowing fast checking for cancellation seem important. E.g. if we want to cancel in the middle of parsing, we'll probably be faced with sema callbacks, frequency of which we don't control, so we'd better design something that's fast to check. I see two ways to deal with this: - Keep the current design, but cache last access to context key. - Add an API to get something that can quickly check for cancellation, i.e. something that would hold a reference to resolved context key. Both could be added later, though, and I don't have any data, it's just my intuition. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51996: [clangd] Simplify cancellation public API
sammccall added a comment. In https://reviews.llvm.org/D51996#1232459, @kadircet wrote: > Seems a lot cleaner now, thanks! > > Do you plan to have other changes like moving control to JSONRPCDispatcher > and recording timings for analysis on this patch? https://reviews.llvm.org/D52004 is the JSONRPCDispatcher change. Added a FIXME to the file comment about timings, hope to get to it soon. > If not maybe we can add some fixme's so that we won't forget. Also the > somewhat "caching" of cancellation token from previous implementation might > still be useful in future if we really face "crowded" contexts and frequent > cancellation checks, so maybe keep some notes about it? I think the best plan for frequent cancellation checks is "don't do that" ;-) The situation is that you're checking for cancellation to save CPU, but the thing you're spending the CPU on is checking for cancellation... I don't think the fix is to make cancellation checking cheaper! (Obviously cheaper is always better, but it would make the API worse here) So i left a note on isCancelled(). We can revisit if we find cases where context traversal is really hurting us. Comment at: clangd/Cancellation.h:29 +// 2. Library code that executes long-running work, and can exit early if the +// result is not needed. // kadircet wrote: > Maybe also mention propagating context into long-running work. runAsync does > that implicitly and not sure if there will be other use cases that doesn't > include it, but if there might be it would be nice to point it out as well. Yeah, added this to the caveats. This shouldn't be a problem in practice (at least for pure open-source clangd): LLVM doesn't have a lot of multithreading utilities so we mostly define our own, and those are context aware. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51996: [clangd] Simplify cancellation public API
sammccall updated this revision to Diff 165158.
sammccall marked 2 inline comments as done.
sammccall added a comment.
Address comments by adding comments!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51996
Files:
clangd/Cancellation.cpp
clangd/Cancellation.h
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/CancellationTests.cpp
Index: unittests/clangd/CancellationTests.cpp
===
--- unittests/clangd/CancellationTests.cpp
+++ unittests/clangd/CancellationTests.cpp
@@ -13,57 +13,48 @@
namespace {
TEST(CancellationTest, CancellationTest) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
+ auto Task = cancelableTask();
+ WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
- TH->cancel();
+ Task.second();
EXPECT_TRUE(isCancelled());
}
-TEST(CancellationTest, TaskTestHandleDiesContextLives) {
+TEST(CancellationTest, CancelerDiesContextLives) {
llvm::Optional ContextWithCancellation;
{
-TaskHandle TH = Task::createHandle();
-ContextWithCancellation.emplace(setCurrentTask(TH));
+auto Task = cancelableTask();
+ContextWithCancellation.emplace(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
EXPECT_TRUE(isCancelled());
}
TEST(CancellationTest, TaskContextDiesHandleLives) {
- TaskHandle TH = Task::createHandle();
+ auto Task = cancelableTask();
{
-WithContext ContextWithCancellation(setCurrentTask(TH));
+WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
// Still should be able to cancel without any problems.
- TH->cancel();
-}
-
-TEST(CancellationTest, CancellationToken) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
- const auto = getCurrentTask();
- EXPECT_FALSE(CT.isCancelled());
- TH->cancel();
- EXPECT_TRUE(CT.isCancelled());
+ Task.second();
}
TEST(CancellationTest, AsynCancellationTest) {
std::atomic HasCancelled(false);
Notification Cancelled;
- auto TaskToBeCancelled = [&](ConstTaskHandle CT) {
-WithContext ContextGuard(setCurrentTask(std::move(CT)));
+ auto TaskToBeCancelled = [&](Context Ctx) {
+WithContext ContextGuard(std::move(Ctx));
Cancelled.wait();
HasCancelled = isCancelled();
};
- TaskHandle TH = Task::createHandle();
- std::thread AsyncTask(TaskToBeCancelled, TH);
- TH->cancel();
+ auto Task = cancelableTask();
+ std::thread AsyncTask(TaskToBeCancelled, std::move(Task.first));
+ Task.second();
Cancelled.notify();
AsyncTask.join();
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -125,9 +125,9 @@
/// while returned future is not yet ready.
/// A version of `codeComplete` that runs \p Callback on the processing thread
/// when codeComplete results become available.
- TaskHandle codeComplete(PathRef File, Position Pos,
- const clangd::CodeCompleteOptions ,
- Callback CB);
+ Canceler codeComplete(PathRef File, Position Pos,
+const clangd::CodeCompleteOptions ,
+Callback CB);
/// Provide signature help for \p File at \p Pos. This method should only be
/// called for tracked files.
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -201,16 +201,16 @@
WorkScheduler.remove(File);
}
-TaskHandle ClangdServer::codeComplete(PathRef File, Position Pos,
- const clangd::CodeCompleteOptions ,
- Callback CB) {
+Canceler ClangdServer::codeComplete(PathRef File, Position Pos,
+const clangd::CodeCompleteOptions ,
+Callback CB) {
// Copy completion options for passing them to async task handler.
auto CodeCompleteOpts = Opts;
if (!CodeCompleteOpts.Index) // Respect overridden index.
CodeCompleteOpts.Index = Index;
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
+ auto Cancelable = cancelableTask();
+ WithContext ContextWithCancellation(std::move(Cancelable.first));
// Copy PCHs to avoid accessing this->PCHs concurrently
std::shared_ptr PCHs = this->PCHs;
auto FS = FSProvider.getFileSystem();
@@ -259,7 +259,7 @@
// We use a potentially-stale preamble because latency is critical here.
WorkScheduler.runWithPreamble("CodeComplete", File,
[PATCH] D51996: [clangd] Simplify cancellation public API
kadircet added a comment. Seems a lot cleaner now, thanks! Do you plan to have other changes like moving control to JSONRPCDispatcher and recording timings for analysis on this patch? If not maybe we can add some fixme's so that we won't forget. Also the somewhat "caching" of cancellation token from previous implementation might still be useful in future if we really face "crowded" contexts and frequent cancellation checks, so maybe keep some notes about it? Comment at: clangd/Cancellation.h:29 +// 2. Library code that executes long-running work, and can exit early if the +// result is not needed. // Maybe also mention propagating context into long-running work. runAsync does that implicitly and not sure if there will be other use cases that doesn't include it, but if there might be it would be nice to point it out as well. Comment at: clangd/Cancellation.h:44 +// (A real example may invoke the callback with an error on cancellation, +// the TaskCancelledError is provided for this purpose). // s/TaskCancelledError/CancelledError Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51996: [clangd] Simplify cancellation public API
sammccall updated this revision to Diff 165125.
sammccall added a comment.
Fix includes, formatting tweak.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51996
Files:
clangd/Cancellation.cpp
clangd/Cancellation.h
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/CancellationTests.cpp
Index: unittests/clangd/CancellationTests.cpp
===
--- unittests/clangd/CancellationTests.cpp
+++ unittests/clangd/CancellationTests.cpp
@@ -13,57 +13,48 @@
namespace {
TEST(CancellationTest, CancellationTest) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
+ auto Task = cancelableTask();
+ WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
- TH->cancel();
+ Task.second();
EXPECT_TRUE(isCancelled());
}
-TEST(CancellationTest, TaskTestHandleDiesContextLives) {
+TEST(CancellationTest, CancelerDiesContextLives) {
llvm::Optional ContextWithCancellation;
{
-TaskHandle TH = Task::createHandle();
-ContextWithCancellation.emplace(setCurrentTask(TH));
+auto Task = cancelableTask();
+ContextWithCancellation.emplace(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
EXPECT_TRUE(isCancelled());
}
TEST(CancellationTest, TaskContextDiesHandleLives) {
- TaskHandle TH = Task::createHandle();
+ auto Task = cancelableTask();
{
-WithContext ContextWithCancellation(setCurrentTask(TH));
+WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
// Still should be able to cancel without any problems.
- TH->cancel();
-}
-
-TEST(CancellationTest, CancellationToken) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
- const auto = getCurrentTask();
- EXPECT_FALSE(CT.isCancelled());
- TH->cancel();
- EXPECT_TRUE(CT.isCancelled());
+ Task.second();
}
TEST(CancellationTest, AsynCancellationTest) {
std::atomic HasCancelled(false);
Notification Cancelled;
- auto TaskToBeCancelled = [&](ConstTaskHandle CT) {
-WithContext ContextGuard(setCurrentTask(std::move(CT)));
+ auto TaskToBeCancelled = [&](Context Ctx) {
+WithContext ContextGuard(std::move(Ctx));
Cancelled.wait();
HasCancelled = isCancelled();
};
- TaskHandle TH = Task::createHandle();
- std::thread AsyncTask(TaskToBeCancelled, TH);
- TH->cancel();
+ auto Task = cancelableTask();
+ std::thread AsyncTask(TaskToBeCancelled, std::move(Task.first));
+ Task.second();
Cancelled.notify();
AsyncTask.join();
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -125,9 +125,9 @@
/// while returned future is not yet ready.
/// A version of `codeComplete` that runs \p Callback on the processing thread
/// when codeComplete results become available.
- TaskHandle codeComplete(PathRef File, Position Pos,
- const clangd::CodeCompleteOptions ,
- Callback CB);
+ Canceler codeComplete(PathRef File, Position Pos,
+const clangd::CodeCompleteOptions ,
+Callback CB);
/// Provide signature help for \p File at \p Pos. This method should only be
/// called for tracked files.
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -201,16 +201,16 @@
WorkScheduler.remove(File);
}
-TaskHandle ClangdServer::codeComplete(PathRef File, Position Pos,
- const clangd::CodeCompleteOptions ,
- Callback CB) {
+Canceler ClangdServer::codeComplete(PathRef File, Position Pos,
+const clangd::CodeCompleteOptions ,
+Callback CB) {
// Copy completion options for passing them to async task handler.
auto CodeCompleteOpts = Opts;
if (!CodeCompleteOpts.Index) // Respect overridden index.
CodeCompleteOpts.Index = Index;
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
+ auto Cancelable = cancelableTask();
+ WithContext ContextWithCancellation(std::move(Cancelable.first));
// Copy PCHs to avoid accessing this->PCHs concurrently
std::shared_ptr PCHs = this->PCHs;
auto FS = FSProvider.getFileSystem();
@@ -259,7 +259,7 @@
// We use a potentially-stale preamble because latency is critical here.
WorkScheduler.runWithPreamble("CodeComplete", File, TUScheduler::Stale,
[PATCH] D51996: [clangd] Simplify cancellation public API
sammccall created this revision.
sammccall added reviewers: ilya-biryukov, kadircet.
Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay, ioeric.
Task is no longer exposed:
- task cancellation is hidden as a std::function
- task creation returns the new context directly
- checking is via free function only, with no way to avoid the context lookup
The implementation is essentially the same, but a bit terser as it's hidden.
isCancelled() is now safe to use outside any task (it returns false).
This will leave us free to sprinkle cancellation in e.g. TUScheduler without
needing elaborate test setup, and lets callers that don't cancel "just work".
Updated the docs to describe the new expected use pattern.
One thing I noticed: there's nothing async-specific about the cancellation.
Async tasks can be cancelled from any thread (typically the one that created
them), sync tasks can be cancelled from any *other* thread in the same way.
So the docs now refer to "long-running" tasks instead of async ones.
Updated usage in code complete, without any structural changes.
I didn't update all the names of the helpers in ClangdLSPServer (these will
likely be moved to JSONRPCDispatcher anyway).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51996
Files:
clangd/Cancellation.cpp
clangd/Cancellation.h
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/CancellationTests.cpp
Index: unittests/clangd/CancellationTests.cpp
===
--- unittests/clangd/CancellationTests.cpp
+++ unittests/clangd/CancellationTests.cpp
@@ -13,57 +13,48 @@
namespace {
TEST(CancellationTest, CancellationTest) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
+ auto Task = cancelableTask();
+ WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
- TH->cancel();
+ Task.second();
EXPECT_TRUE(isCancelled());
}
-TEST(CancellationTest, TaskTestHandleDiesContextLives) {
+TEST(CancellationTest, CancelerDiesContextLives) {
llvm::Optional ContextWithCancellation;
{
-TaskHandle TH = Task::createHandle();
-ContextWithCancellation.emplace(setCurrentTask(TH));
+auto Task = cancelableTask();
+ContextWithCancellation.emplace(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
EXPECT_TRUE(isCancelled());
}
TEST(CancellationTest, TaskContextDiesHandleLives) {
- TaskHandle TH = Task::createHandle();
+ auto Task = cancelableTask();
{
-WithContext ContextWithCancellation(setCurrentTask(TH));
+WithContext ContextWithCancellation(std::move(Task.first));
EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
EXPECT_TRUE(isCancelled());
}
// Still should be able to cancel without any problems.
- TH->cancel();
-}
-
-TEST(CancellationTest, CancellationToken) {
- TaskHandle TH = Task::createHandle();
- WithContext ContextWithCancellation(setCurrentTask(TH));
- const auto = getCurrentTask();
- EXPECT_FALSE(CT.isCancelled());
- TH->cancel();
- EXPECT_TRUE(CT.isCancelled());
+ Task.second();
}
TEST(CancellationTest, AsynCancellationTest) {
std::atomic HasCancelled(false);
Notification Cancelled;
- auto TaskToBeCancelled = [&](ConstTaskHandle CT) {
-WithContext ContextGuard(setCurrentTask(std::move(CT)));
+ auto TaskToBeCancelled = [&](Context Ctx) {
+WithContext ContextGuard(std::move(Ctx));
Cancelled.wait();
HasCancelled = isCancelled();
};
- TaskHandle TH = Task::createHandle();
- std::thread AsyncTask(TaskToBeCancelled, TH);
- TH->cancel();
+ auto Task = cancelableTask();
+ std::thread AsyncTask(TaskToBeCancelled, std::move(Task.first));
+ Task.second();
Cancelled.notify();
AsyncTask.join();
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -125,9 +125,9 @@
/// while returned future is not yet ready.
/// A version of `codeComplete` that runs \p Callback on the processing thread
/// when codeComplete results become available.
- TaskHandle codeComplete(PathRef File, Position Pos,
- const clangd::CodeCompleteOptions ,
- Callback CB);
+ Canceler codeComplete(PathRef File, Position Pos,
+const clangd::CodeCompleteOptions ,
+Callback CB);
/// Provide signature help for \p File at \p Pos. This method should only be
/// called for tracked files.
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -201,16 +201,16 @@
WorkScheduler.remove(File);
}
-TaskHandle
