[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0f287464d8a: [clangd] Add $/memoryUsage LSP extension 
(authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D89277?vs=298644=298990#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/memory_tree.test

Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -0,0 +1,81 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"$/memoryUsage","params":{}}
+# CHECK:"id": 1,
+# CHECK-NEXT:   "jsonrpc": "2.0",
+# CHECK-NEXT:   "result": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "clangd_server": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "dynamic_index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "main_file": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "references": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "relations": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "preamble": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "tuscheduler": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "ast": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "preamble": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -66,6 +66,7 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
+# CHECK-NEXT:  "memoryUsageProvider": true
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -25,6 +25,7 @@
 
 

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 298644.
kadircet added a comment.

s/memoryTree/memoryUsage/g


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/memory_tree.test

Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -0,0 +1,81 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"$/memoryTree","params":{}}
+# CHECK:"id": 1,
+# CHECK-NEXT:   "jsonrpc": "2.0",
+# CHECK-NEXT:   "result": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "clangd_server": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "dynamic_index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "main_file": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "references": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "relations": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "preamble": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "tuscheduler": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "ast": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "preamble": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -66,6 +66,7 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
+# CHECK-NEXT:  "memoryTreeProvider": true
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -25,6 +25,7 @@
 
 #include "URI.h"
 #include "index/SymbolID.h"
+#include "support/MemoryTree.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/JSON.h"
@@ -1576,6 +1577,27 @@
 };
 

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Sam McCall via Phabricator via cfe-commits]

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:623
 {"typeHierarchyProvider", true},
+{"memoryTreeProvider", true}, // clangd extension.
 ;

one last naming nit: I think `memoryUsage` would put appropriately more weight 
on the semantics rather than the data structure


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 298578.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Add memoryTreeProvider to initialization params
- Move serialization logic to Protocol.h and return MemoryTree directly.
- Drop `dump` from method name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/memory_tree.test

Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -0,0 +1,81 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"$/memoryTree","params":{}}
+# CHECK:"id": 1,
+# CHECK-NEXT:   "jsonrpc": "2.0",
+# CHECK-NEXT:   "result": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "clangd_server": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "dynamic_index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "main_file": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "references": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "relations": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "preamble": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "tuscheduler": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "ast": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "preamble": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -66,6 +66,7 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
+# CHECK-NEXT:  "memoryTreeProvider": true
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -25,6 +25,7 @@
 
 #include "URI.h"
 #include 

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks really nice!




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:615
 {"workspaceSymbolProvider", true},
 {"referencesProvider", true},
 {"executeCommandProvider",

can you add `{"memoryTreeProvider", true} // clangd extension`?

I'd like to add client support in VSCode and it's useful if the command is only 
visible with versions of clangd that support it.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1389
 
+void ClangdLSPServer::onDumpMemoryTree(const NoParams &,
+   Callback Reply) {

Putting the serialization code inline is a tempting option here because it's 
(mostly) just a map.
It's not something we do often though, so it does bury the protocol details in 
an unexpected place.

A couple of alternatives to consider:
 - map to a `struct UsageTree { unsigned Self; unsigned Total; Map Children; }` defined in protocol.h, and define marshalling in the 
usual way
 - skip the struct and use MemoryTree as the type, providing a toJSON(const 
MemoryTree&) function in protocol.h

I lean towards the last alternative because it provides a bit more regularity 
without adding much more redundancy, but this is up to you.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1391
+   Callback Reply) {
+  llvm::BumpPtrAllocator Alloc;
+  MemoryTree MT();

nit: Alloc -> DetailAlloc



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1458
   MsgHandler->bind("textDocument/semanticTokens/full/delta", 
::onSemanticTokensDelta);
+  MsgHandler->bind("$/dumpMemoryTree", ::onDumpMemoryTree);
   if (Opts.FoldingRanges)

I think "dump" is a bit casual and also redundant - LSP methods to obtain 
information tend to have noun names. (executeCommand is the exception that 
proves the rule - it's used for side effects).

I went through the same question with the AST-dumping and landed on 
textDocument/ast as my preferred option, for what it's worth.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:145
  Callback);
+  /// This is a clangd extension. Provides a json tree representing memory 
usage
+  /// hierarchy. Keys starting with an underscore(_) represent leaves, e.g.

(if you decide to move toJSON to protocol.h, then the second sentence onward 
probably belongs there)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 298460.
kadircet added a comment.

- As discussed offline moving with compact version of the output, while 
preserving names starting with an `_` to be leaves.

This is also ready for an implementation-wise review now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/memory_tree.test

Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -0,0 +1,81 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"$/dumpMemoryTree","params":{}}
+# CHECK:"id": 1,
+# CHECK-NEXT:   "jsonrpc": "2.0",
+# CHECK-NEXT:   "result": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "clangd_server": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "dynamic_index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "main_file": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "references": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "relations": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "preamble": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "tuscheduler": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "ast": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "preamble": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -24,6 +24,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/JSON.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -141,6 +142,27 @@
   void onSemanticTokens(const SemanticTokensParams &, Callback);
   void onSemanticTokensDelta(const SemanticTokensDeltaParams &,
  Callback);
+  /// This is a clangd extension. Provides a json tree representing memory usage
+  /// hierarchy. Keys starting with an underscore(_) represent leaves, e.g.
+  /// _total or _self for memory usage of whole subtree or only that specific
+  /// node in bytes. All other keys represents children. An example:
+  ///   {
+  /// "_self": 0,
+  /// "_total": 8,
+  /// "child1": {
+  ///

Re: [PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Sam McCall via cfe-commits]

On Wed, Oct 14, 2020 at 6:10 PM Kadir Cetinkaya via Phabricator <
revi...@reviews.llvm.org> wrote:

> kadircet added a comment.
>
> In D89277#2329947 , @sammccall
> wrote:
>
> > (sorry out today and haven't looked at code yet)
>
> no worries it is a prototype, I wouldn't spend time looking at the
> implementation until we agree on the interaction :D
> OTHO, checking out the lit test for output would probably be useful.
>
> > If it's a custom method, I think it should return the data as a json
> structure - the client already has to have custom support to invoke it,
> displaying the result isn't much extra work.
>
> SGTM. WDYT about a json object in the form of:
>
>   interface MemoryTreeNode {
> name: string;
> totalSize: int;
>
I'd also include self-size.


> children?: Node[];
>   };
>
This looks like:
{
  "name": "root"
  "totalSize": 100,
  "children": [
{
  "name": "component1",
  "totalSize": 25,
},
{
  "name": "component2",
  "totalSize": 75,
}
  ]
}

A couple of alternatives: I think making "children" an object and moving
the name up to the edge is more natural.
We no longer name the root, which I don't think is a bad thing.
{
  "totalSize": 100,
  "children": {
"component1": {
  "totalSize": 25,
},
"component2": {
  "totalSize": 75,
}
  ]
}

A more-compact alternative folds the child attributes into the node itself.
This is probably more awkward to query directly (e.g. "does a node have
children") and might end up being marshalled into a struct.
On the other hand, I think it's easier to read directly e.g. in logs, esp
when deeply nested.
{
  "_total": 100,
  "component1": {
"_total": 25
  }
  "component2": {
"_total": 25
  }
}

> And I would really love to add a tree view to vscode, I think it wouldn't
> be hard (vs call hierarchy: no laziness and no direction-flipping) and
> could be reused for an AST viewer.
>
> right, vscode already has APIs for it,
> https://code.visualstudio.com/api/extension-guides/tree-view.
>
I found some time today and have a prototype of AST dump... nearly working
:-)
The complexity of the treeview API does indeed mostly disappear when you
have simple tree-shaped data and it's all available at once.
Just the annoying extension-point boilerplate remains...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D89277#2329947 , @sammccall wrote:

> (sorry out today and haven't looked at code yet)

no worries it is a prototype, I wouldn't spend time looking at the 
implementation until we agree on the interaction :D
OTHO, checking out the lit test for output would probably be useful.

> If it's a custom method, I think it should return the data as a json 
> structure - the client already has to have custom support to invoke it, 
> displaying the result isn't much extra work.

SGTM. WDYT about a json object in the form of:

  interface MemoryTreeNode {
name: string;
totalSize: int;
children?: Node[];
  };



> And I would really love to add a tree view to vscode, I think it wouldn't be 
> hard (vs call hierarchy: no laziness and no direction-flipping) and could be 
> reused for an AST viewer.

right, vscode already has APIs for it, 
https://code.visualstudio.com/api/extension-guides/tree-view.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(sorry out today and haven't looked at code yet)

If it's a custom method, I think it should return the data as a json structure 
- the client already has to have custom support to invoke it, displaying the 
result isn't much extra work.

And I would really love to add a tree view to vscode, I think it wouldn't be 
hard (vs call hierarchy: no laziness and no direction-flipping) and could be 
reused for an AST viewer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D89277#2326376 , @nridge wrote:

> This is neat! Are there plans to add vscode client support to invoke this?

Yes, since we control the plugin in vscode we should have a way to invoke it 
there. (we already have support for `switchHeaderSource`).

We should first agree on the interaction though. This currently uses 
`showMessage` to display the dump, which is the most generic way LSP provides 
us with, but usually isn't good for multi-line output.
Other options we have are:

- Write it into logs - Hard to find, but at least will look reasonable on every 
editor.
- Insert as a commented block into the code. (like dumpast) - Clutters the 
code, but again looks reasonable on every editor and easier to find than logs.
- Provide a json object in clangd and use fancy client side features like tree 
views. - Probably the best output, but will only work on a handful of editors 
and will require a lot of effort to implement.

I am leaning towards using showMessage and also logging the tree. Do others 
have opinions on other possibilities or pros/cons of the options i've listed 
above?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

This is neat! Are there plans to add vscode client support to invoke this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89277/new/

https://reviews.llvm.org/D89277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Performs a detailed profiling of clangd lsp server and conveys the
result to the client via showMessage.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89277

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/memory_tree.test

Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -0,0 +1,33 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"$/dumpMemoryTree","params":{}}
+# CHECK:  "method": "window/showMessage",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "message": "
+# CHECK-SAME:   clangd_lsp_server: {{[0-9]+}} bytes\n
+# CHECK-SAME: clangd_server: {{[0-9]+}} bytes\n
+# CHECK-SAME:   tuscheduler: {{[0-9]+}} bytes\n
+# CHECK-SAME: /clangd-test/main.cpp: {{[0-9]+}} bytes\n
+# CHECK-SAME:   ast: {{[0-9]+}} bytes\n
+# CHECK-SAME:   preamble: {{[0-9]+}} bytes\n
+# CHECK-SAME:   dynamic_index: {{[0-9]+}} bytes\n
+# CHECK-SAME: preamble: {{[0-9]+}} bytes\n
+# CHECK-SAME:   symbols: {{[0-9]+}} bytes\n
+# CHECK-SAME:   index: {{[0-9]+}} bytes\n
+# CHECK-SAME: main_file: {{[0-9]+}} bytes\n
+# CHECK-SAME:   symbols: {{[0-9]+}} bytes\n
+# CHECK-SAME: /clangd-test/main.cpp: {{[0-9]+}} bytes\n
+# CHECK-SAME:   relations: {{[0-9]+}} bytes\n
+# CHECK-SAME:   references: {{[0-9]+}} bytes\n
+# CHECK-SAME:   symbols: {{[0-9]+}} bytes\n
+# CHECK-SAME:   index: {{[0-9]+}} bytes"
+# CHECK-NEXT:"type": 3
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -24,6 +24,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/JSON.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -141,6 +142,9 @@
   void onSemanticTokens(const SemanticTokensParams &, Callback);
   void onSemanticTokensDelta(const SemanticTokensDeltaParams &,
  Callback);
+  /// This is a clangd extension. It invokes a showMessage with current
+  /// profiling info.
+  void onDumpMemoryTree(const NoParams &, Callback);
 
   std::vector getFixes(StringRef File, const clangd::Diagnostic );
 
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -36,8 +36,11 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1383,6 +1386,38 @@
   });
 }
 
+void ClangdLSPServer::onDumpMemoryTree(const NoParams &,
+   Callback Reply) {
+  llvm::BumpPtrAllocator Alloc;
+  MemoryTree MT();
+  profile(MT);
+
+  std::function
+  DumpTree = [&](llvm::StringRef Key, const MemoryTree ,
+ llvm::raw_ostream , size_t Indent) {
+size_t Total = MT.self();
+std::string ChildText;
+llvm::raw_string_ostream ChildStream(ChildText);
+for (const auto  : MT.children()) {
+  ChildStream << '\n';
+  Total +=
+  DumpTree(Entry.first, Entry.getSecond(), ChildStream, Indent + 2);
+}
+OS << std::string(Indent, ' ') << Key << ": " << Total << " bytes"
+   << ChildStream.str();
+return Total;
+  };
+
+  ShowMessageParams Msg;
+  llvm::raw_string_ostream Str(Msg.message);
+  DumpTree("clangd_lsp_server", MT, Str, 0);
+  Str.flush();
+
+  notify("window/showMessage", std::move(Msg));
+  Reply(nullptr);
+}
+
 ClangdLSPServer::ClangdLSPServer(class Transport ,
  const ThreadsafeFS ,
  const ClangdLSPServer::Options )
@@ -1425,6 +1460,7 @@
   MsgHandler->bind("textDocument/documentLink", ::onDocumentLink);