sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1234
+void ClangdLSPServer::profile(bool Detailed) {
+ if (!trace::enabled())
----------------
ClangdLSPServer does own things other than ClangdServer (e.g. the CDB), though
we don't yet profile them.
Does it make sense to split this into ClangdLSPServer::profile (with the usual
signature, and maybe public?) and ClangdLSPServer::maybeExportMemoryProfile()?
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1234
+void ClangdLSPServer::profile(bool Detailed) {
+ if (!trace::enabled())
----------------
sammccall wrote:
> ClangdLSPServer does own things other than ClangdServer (e.g. the CDB),
> though we don't yet profile them.
> Does it make sense to split this into ClangdLSPServer::profile (with the
> usual signature, and maybe public?) and
> ClangdLSPServer::maybeExportMemoryProfile()?
Detailed is always false, drop the parameter? This can't be reused by code
wanting to e.g. service a code action anyway.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1424
+
+ // Delay profiling for a minute, as serious allocations don't happen at
+ // startup.
----------------
As written this isn't clearly true or false (what exactly is "startup"?)
Maybe just "Delay first profile until we've finished warming up"?
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:165
+ /// requests.
+ std::chrono::steady_clock::time_point NoProfileBefore;
+
----------------
or NextProfileTime? (to avoid the negation)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88417/new/
https://reviews.llvm.org/D88417
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
