[PATCH] D88417: [clangd] Record memory usages after each notification

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG35871fde55ac: [clangd] Record memory usages after each notification (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet updated this revision to Diff 297568. kadircet marked an inline comment as done. kadircet added a comment. - Update stale comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files:

[PATCH] D88417: [clangd] Record memory usages after each notification

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:72 + /// Profiles resource-usage. No-op if there's no active tracer. + void profile(MemoryTree ) const;

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet updated this revision to Diff 297565. kadircet marked 5 inline comments as done. kadircet added a comment. - Separate profiling and exporting into 2 functions - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/

[PATCH] D88417: [clangd] Record memory usages after each notification

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

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet updated this revision to Diff 297173. kadircet marked 2 inline comments as done. kadircet added a comment. - Implement periodic profiling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files:

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet added a comment. > What about something like a 5 minute throttle, but have ClangdLSPServer's > constructor set the timestamp to now+1 minute? (Without profiling) SGTM. Note that this means we can't easily test this in LSP layer anymore. (We've got couple of components depending on

[PATCH] D88417: [clangd] Record memory usages after each notification

sammccall added a comment. In D88417#2319307 , @kadircet wrote: > Bad news, I was testing this with remote-index, hence background-index was > turned off. Unfortunately traversing all of the slabs in `FileSymbols` takes > quite a while in this case

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet requested review of this revision. kadircet added a comment. Bad news, I was testing this with remote-index, hence background-index was turned off. Unfortunately traversing all of the slabs in `FileSymbols` takes quite a while in this case (~15ms for LLVM). I don't think it is

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet updated this revision to Diff 296955. kadircet marked 4 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files:

[PATCH] D88417: [clangd] Record memory usages after each notification

sammccall added a comment. this is at least going to take some important locks, hopefully only briefly. We should watch the timing here carefully and consider guarding it - apart from the minimum time interval we discussed, we could have a check whether metric tracing is actually enabled in a

[PATCH] D88417: [clangd] Record memory usages after each notification

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:182 +Server.Server->profile(MT); +trace::recordMemoryUsage(MT, "clangd_server"); return true;

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet updated this revision to Diff 296696. kadircet added a comment. - Rebase and add tests for ClangdLSPServer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files:

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet updated this revision to Diff 296115. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88417/new/ https://reviews.llvm.org/D88417 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D88417: [clangd] Record memory usages after each notification

kadircet created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Depends on D88415 Repository: rG LLVM