[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG32f102912493: [clangd] Track performance of IncludeCleaner 
(authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

oops forgot to LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

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

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 383278.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:275
 std::vector computeUnusedIncludes(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto &SM = AST.getSourceManager();

this doesn't look interesting on its own, as it doesn't perform much magic. 
maybe just drop?



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:284
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {

i believe this is the function that deserves a trace the most, as it is alive 
throughout the whole interaction and will reflect full latency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

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

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112783

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -269,6 +272,7 @@
 }
 
 std::vector computeUnusedIncludes(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto &SM = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -269,6 +272,7 @@
 }
 
 std::vector computeUnusedIncludes(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto &SM = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits