[PATCH] D123672: [clangd] Export preamble AST and serialized size as metrics

Wed Apr 13 05:39:42 PDT 2022

sammccall added inline comments.

================
Comment at: clang-tools-extra/clangd/Preamble.cpp:500

+  if (Stats != nullptr) {
+    Stats->TotalBuildTime = PreambleTimer.getTime();
----------------
adamcz wrote:
> This is a significant change. You are now exporting this information for failed preamble builds. Do you think these are interesting? It might make the data a bit confusing (i.e. lots of small "built in <1s, did not read any files) when something is really wrong with preamble.
> 
> I think we should either only export on successful preamble OR add a boolean "success" dimension to the metric.
This doesn't change what is exported, that's still controlled by TUScheduler in the same way (only exporting on success).

It just changes the behavior of this function: it now populates stats instead of leaving them uninitialized. The function already returns a success dimension (either the returned pointer is null or not).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123672