[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-13 Thread Sam McCall via Phabricator via cfe-commits
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&id=165251#toc Repository: rCTE Clang Too

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-13 Thread Sam McCall via Phabricator via cfe-commits
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 pra

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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://lis

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-13 Thread Ilya Biryukov via Phabricator via cfe-commits
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

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-13 Thread Ilya Biryukov via Phabricator via cfe-commits
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: c

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-12 Thread Sam McCall via Phabricator via cfe-commits
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 J

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-12 Thread Sam McCall via Phabricator via cfe-commits
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/Cla

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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 toke

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-12 Thread Sam McCall via Phabricator via cfe-commits
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 cla

[PATCH] D51996: [clangd] Simplify cancellation public API

2018-09-12 Thread Sam McCall via Phabricator via cfe-commits
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 - check