[PATCH] D39086: Performance tracing facility for clangd.

2020-10-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/trunk/unittests/clangd/TraceTests.cpp:57 +} +std::string VS = V->getValue(Tmp).str(); +if (VS != I->second) { RKSimon wrote: > @sammccall PVS Studio is reporting a potential null

[PATCH] D39086: Performance tracing facility for clangd.

2020-10-27 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added inline comments. Herald added subscribers: usaxena95, kadircet, arphaman, MaskRay. Comment at: clang-tools-extra/trunk/unittests/clangd/TraceTests.cpp:57 +} +std::string VS = V->getValue(Tmp).str(); +if (VS != I->second) { @sammccall

[PATCH] D39086: Performance tracing facility for clangd.

2017-11-02 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL317193: Performance tracing facility for clangd. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D39086?vs=121253=121257#toc Repository: rL LLVM

[PATCH] D39086: Performance tracing facility for clangd.

2017-11-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 121253. sammccall added a comment. clang-format https://reviews.llvm.org/D39086 Files: clangd/CMakeLists.txt clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/ProtocolHandlers.cpp clangd/Trace.cpp

[PATCH] D39086: Performance tracing facility for clangd.

2017-11-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/Trace.h:38 + // Starts a sessions capturing trace events and writing Trace Event JSON. + static std::unique_ptr createJSON(llvm::raw_ostream ); + ~Session(); ioeric wrote: > `createJSON` is a bit

[PATCH] D39086: Performance tracing facility for clangd.

2017-11-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 121252. sammccall marked an inline comment as done. sammccall added a comment. createJSON -> create https://reviews.llvm.org/D39086 Files: clangd/CMakeLists.txt clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp

[PATCH] D39086: Performance tracing facility for clangd.

2017-11-02 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. still lgtm Comment at: clangd/Trace.h:38 + // Starts a sessions capturing trace events and writing Trace Event JSON. + static std::unique_ptr createJSON(llvm::raw_ostream ); + ~Session(); `createJSON`

[PATCH] D39086: Performance tracing facility for clangd.

2017-11-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 121247. sammccall added a comment. Provide RAII-like interface to trace functionality. Note that we may want to provide a backend API here. https://reviews.llvm.org/D39086 Files: clangd/CMakeLists.txt clangd/ClangdServer.cpp clangd/ClangdUnit.cpp

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clangd/Trace.cpp:87 + +static Tracer* T = nullptr; +} // namespace ilya-biryukov wrote: > Maybe use `unique_ptr` (or even `llvm::Optional`)? Otherwise > we leak memory and

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 119864. sammccall added a comment. Address FIXME now that r316330 is in. https://reviews.llvm.org/D39086 Files: clangd/CMakeLists.txt clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp clangd/ProtocolHandlers.cpp

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/Trace.cpp:87 + +static Tracer* T = nullptr; +} // namespace Maybe use `unique_ptr` (or even `llvm::Optional`)? Otherwise we leak memory and don't flush the stream if users of tracing API forget to call

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D39086 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 119562. sammccall marked 3 inline comments as done. sammccall added a comment. Address review comments. https://reviews.llvm.org/D39086 Files: clangd/CMakeLists.txt clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/JSONRPCDispatcher.cpp

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/JSONRPCDispatcher.cpp:230 - // Finally, execute the action for this JSON message. - if (!Dispatcher.call(JSONRef, Out)) -Out.log("JSON dispatch failed!\n"); + { +// Finally, execute the action

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Looks good in general. A few nits. Comment at: clangd/JSONRPCDispatcher.cpp:230 - // Finally, execute the action for this JSON message. - if (!Dispatcher.call(JSONRef, Out)) -Out.log("JSON dispatch failed!\n"); + { +//

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. Herald added a subscriber: mgorny. This lets you visualize clangd's activity on different threads over time, and understand critical paths of requests and object lifetimes. The data produced can be visualized in Chrome (at chrome://tracing), or in a standalone