[llvm-dwarfdump] --statistics: switch to json::OStream. NFC

[MaskRay]

Summary

Then it is trivial to make the output indented (the second parameter of
json::OStream::OStream specifies the indentation).

[MaskRay]

llvm/tools/llvm-dwarfdump/Statistics.cpp

482

I am not sure repeating the message in LLVM_DEBUG is useful.

I can delete them in a separate change if people think the same as me.

[jhenderson]

Since this is JSON we're trying to emit, using the JSON stream makes sense to me. LGTM, but might be worth hanging on a bit to give others a chance to chime in.

llvm/tools/llvm-dwarfdump/Statistics.cpp

482

Not sure I've got any reasonable opinion here, since I don't really use this code or LLVM_DEBUG output ever. I'd prefer of one of the other typical users chime in here (e.g. @aprantl, @dblaikie, @Orlando)

[Orlando]

llvm/tools/llvm-dwarfdump/Statistics.cpp

482

I'm not sure how much weight my opinion carries here but I agree with removing the prints. I have never used these particular debug prints. When I want to inspect the output I usually just pipe it through python -m json.tool to format it.

[aprantl]

llvm/tools/llvm-dwarfdump/Statistics.cpp

482

I am not sure repeating the message in LLVM_DEBUG is useful.

The original idea was to have human-readable output on stderr and JSON output on stdout. But if this is no longer useful, I'm fine with removing it.

[MaskRay]

llvm/tools/llvm-dwarfdump/Statistics.cpp

482

With

- json::OStream J(OS);
+ json::OStream J(OS, 2);

the JSON will be broken into multiple lines with an indentation of 2. I think that is quite readable, no need for stderr output.

[MaskRay]

Can I get more approval power for this NFC change?

[echristo]

I feel pretty OK approving this.

(Tradeoffs I'd contemplated and discarded was renaming OS rather than OS->J everywhere. I ended up liking the readability of J more. JS might be nice as well?)

-eric

[MaskRay]

In D86045#2227240, @echristo wrote:

I feel pretty OK approving this.

(Tradeoffs I'd contemplated and discarded was renaming OS rather than OS->J everywhere. I ended up liking the readability of J more. JS might be nice as well?)

-eric

Do you mean removing the OS->J rename? That would be a bit ambiguous because people usually use OS to refer to raw_ostream and its derivatives. The JSON stream is special.

[echristo]

In D86045#2227284, @MaskRay wrote:

In D86045#2227240, @echristo wrote:

I feel pretty OK approving this.

(Tradeoffs I'd contemplated and discarded was renaming OS rather than OS->J everywhere. I ended up liking the readability of J more. JS might be nice as well?)

-eric

Do you mean removing the OS->J rename? That would be a bit ambiguous because people usually use OS to refer to raw_ostream and its derivatives. The JSON stream is special.

Yep. And that's one of the reasons why I discarded that one. JS would probably make sense (JSON Stream) if you'd like. No strong opinions.

-eric

[MaskRay]

In D86045#2227367, @echristo wrote:

In D86045#2227284, @MaskRay wrote:

In D86045#2227240, @echristo wrote:

I feel pretty OK approving this.

(Tradeoffs I'd contemplated and discarded was renaming OS rather than OS->J everywhere. I ended up liking the readability of J more. JS might be nice as well?)

-eric

Do you mean removing the OS->J rename? That would be a bit ambiguous because people usually use OS to refer to raw_ostream and its derivatives. The JSON stream is special.

Yep. And that's one of the reasons why I discarded that one. JS would probably make sense (JSON Stream) if you'd like. No strong opinions.

-eric

Thanks! I misread the previous comment (keeping the "OS" name was contemplated but discarded). I'll commit this as is tomorrow if nobody expresses a preference.

llvm/tools/llvm-dwarfdump/Statistics.cpp

collectStatsRecursive(Child, FnPrefix, ChildVarPrefix, BytesInScope,
                           InlineDepth, FnStatMap, GlobalStats, LocStats);
     Child = Child.getSibling();
   }
 }
 
 /// Print machine-readable output.
 /// The machine-readable format is single-line JSON output.
 /// \{
-static void printDatum(raw_ostream &OS, const char *Key, json::Value Value) {
-  OS << ",\"" << Key << "\":" << Value;
+static void printDatum(json::OStream &J, const char *Key, json::Value Value) {
+  J.attribute(Key, Value);
   LLVM_DEBUG(llvm::dbgs() << Key << ": " << Value << '\n');
 }
 
-static void printLocationStats(raw_ostream &OS, const char *Key,
+static void printLocationStats(json::OStream &J, const char *Key,
                                std::vector<unsigned> &LocationStats) {
-  OS << ",\"" << Key << " with 0% of parent scope covered by DW_AT_location\":"
-     << LocationStats[0];
+  J.attribute(
+      (Twine(Key) + " with 0% of parent scope covered by DW_AT_location").str(),
+      LocationStats[0]);
   LLVM_DEBUG(
       llvm::dbgs() << Key
                    << " with 0% of parent scope covered by DW_AT_location: \\"
                    << LocationStats[0] << '\n');
-  OS << ",\"" << Key
-     << " with (0%,10%) of parent scope covered by DW_AT_location\":"
-     << LocationStats[1];
+  J.attribute(
+      (Twine(Key) + " with (0%,10%) of parent scope covered by DW_AT_location")
+          .str(),
+      LocationStats[1]);
   LLVM_DEBUG(llvm::dbgs()
              << Key
              << " with (0%,10%) of parent scope covered by DW_AT_location: "
              << LocationStats[1] << '\n');
   for (unsigned i = 2; i < NumOfCoverageCategories - 1; ++i) {
-    OS << ",\"" << Key << " with [" << (i - 1) * 10 << "%," << i * 10
-       << "%) of parent scope covered by DW_AT_location\":" << LocationStats[i];
+    J.attribute((Twine(Key) + " with [" + Twine((i - 1) * 10) + "%," +
+                 Twine(i * 10) + "%) of parent scope covered by DW_AT_location")
+                    .str(),
+                LocationStats[i]);
     LLVM_DEBUG(llvm::dbgs()
                << Key << " with [" << (i - 1) * 10 << "%," << i * 10
                << "%) of parent scope covered by DW_AT_location: "
                << LocationStats[i]);
   }
-  OS << ",\"" << Key
-     << " with 100% of parent scope covered by DW_AT_location\":"
-     << LocationStats[NumOfCoverageCategories - 1];
+  J.attribute(
+      (Twine(Key) + " with 100% of parent scope covered by DW_AT_location")
+          .str(),
+      LocationStats[NumOfCoverageCategories - 1]);
   LLVM_DEBUG(
       llvm::dbgs() << Key
                    << " with 100% of parent scope covered by DW_AT_location: "
                    << LocationStats[NumOfCoverageCategories - 1]);
 }
 
-static void printSectionSizes(raw_ostream &OS, const SectionSizes &Sizes) {
+static void printSectionSizes(json::OStream &J, const SectionSizes &Sizes) {
   for (const auto &DebugSec : Sizes.DebugSectionSizes)
-    OS << ",\"#bytes in " << DebugSec.getKey() << "\":" << DebugSec.getValue();
+    J.attribute((Twine("#bytes in ") + DebugSec.getKey()).str(),
+                int64_t(DebugSec.getValue()));
 }
 
 /// \}
 
 /// Collect debug info quality metrics for an entire DIContext.
 ///
 /// Do the impossible and reduce the quality of the debug info down to a few
 /// numbers. The idea is to condense the data into numbers that can be tracked
for (auto &Entry : Statistics) {
     LocalVarTotal += Stats.NumLocalVars;
     LocalVarWithType += Stats.NumLocalVarTypes;
     LocalVarWithLoc += Stats.NumLocalVarLocations;
     LocalVarWithSrcLoc += Stats.NumLocalVarSourceLocations;
   }
 
   // Print summary.
   OS.SetBufferSize(1024);
-  OS << "{\"version\":" << Version;
+  json::OStream J(OS);
+  J.objectBegin();
+  J.attribute("version", Version);
   LLVM_DEBUG(llvm::dbgs() << "Variable location quality metrics\n";
              llvm::dbgs() << "---------------------------------\n");
 
-  printDatum(OS, "file", Filename.str());
-  printDatum(OS, "format", FormatName);
+  printDatum(J, "file", Filename.str());
+  printDatum(J, "format", FormatName);
 
-  printDatum(OS, "#functions", NumFunctions);
-  printDatum(OS, "#functions with location", NumFuncsWithSrcLoc);
-  printDatum(OS, "#inlined functions", NumInlinedFunctions);
-  printDatum(OS, "#inlined functions with abstract origins",
-             NumAbstractOrigins);
+  printDatum(J, "#functions", NumFunctions);
+  printDatum(J, "#functions with location", NumFuncsWithSrcLoc);
+  printDatum(J, "#inlined functions", NumInlinedFunctions);
+  printDatum(J, "#inlined functions with abstract origins", NumAbstractOrigins);
 
   // This includes local variables and formal parameters.
-  printDatum(OS, "#unique source variables", VarParamUnique);
-  printDatum(OS, "#source variables", VarParamTotal);
-  printDatum(OS, "#source variables with location", VarParamWithLoc);
-
-  printDatum(OS, "#call site entries", GlobalStats.CallSiteEntries);
-  printDatum(OS, "#call site DIEs", GlobalStats.CallSiteDIEs);
-  printDatum(OS, "#call site parameter DIEs", GlobalStats.CallSiteParamDIEs);
+  printDatum(J, "#unique source variables", VarParamUnique);
+  printDatum(J, "#source variables", VarParamTotal);
+  printDatum(J, "#source variables with location", VarParamWithLoc);
+
+  printDatum(J, "#call site entries", GlobalStats.CallSiteEntries);
+  printDatum(J, "#call site DIEs", GlobalStats.CallSiteDIEs);
+  printDatum(J, "#call site parameter DIEs", GlobalStats.CallSiteParamDIEs);
 
-  printDatum(OS, "sum_all_variables(#bytes in parent scope)",
+  printDatum(J, "sum_all_variables(#bytes in parent scope)",
              GlobalStats.ScopeBytes);
-  printDatum(OS,
+  printDatum(J,
              "sum_all_variables(#bytes in parent scope covered by "
              "DW_AT_location)",
              GlobalStats.ScopeBytesCovered);
-  printDatum(OS,
+  printDatum(J,
              "sum_all_variables(#bytes in parent scope covered by "
              "DW_OP_entry_value)",
              GlobalStats.ScopeEntryValueBytesCovered);
 
-  printDatum(OS, "sum_all_params(#bytes in parent scope)",
+  printDatum(J, "sum_all_params(#bytes in parent scope)",
              GlobalStats.ParamScopeBytes);
-  printDatum(
-      OS,
+  printDatum(J,
              "sum_all_params(#bytes in parent scope covered by DW_AT_location)",
              GlobalStats.ParamScopeBytesCovered);
-  printDatum(OS,
+  printDatum(J,
              "sum_all_params(#bytes in parent scope covered by "
              "DW_OP_entry_value)",
              GlobalStats.ParamScopeEntryValueBytesCovered);
 
-  printDatum(OS, "sum_all_local_vars(#bytes in parent scope)",
+  printDatum(J, "sum_all_local_vars(#bytes in parent scope)",
              GlobalStats.LocalVarScopeBytes);
-  printDatum(OS,
+  printDatum(J,
              "sum_all_local_vars(#bytes in parent scope covered by "
              "DW_AT_location)",
              GlobalStats.LocalVarScopeBytesCovered);
-  printDatum(OS,
+  printDatum(J,
              "sum_all_local_vars(#bytes in parent scope covered by "
              "DW_OP_entry_value)",
              GlobalStats.LocalVarScopeEntryValueBytesCovered);
 
-  printDatum(OS, "#bytes witin functions", GlobalStats.FunctionSize);
-  printDatum(OS, "#bytes witin inlined functions",
+  printDatum(J, "#bytes witin functions", GlobalStats.FunctionSize);
+  printDatum(J, "#bytes witin inlined functions",
              GlobalStats.InlineFunctionSize);
 
   // Print the summary for formal parameters.
-  printDatum(OS, "#params", ParamTotal);
-  printDatum(OS, "#params with source location", ParamWithSrcLoc);
-  printDatum(OS, "#params with type", ParamWithType);
-  printDatum(OS, "#params with binary location", ParamWithLoc);
+  printDatum(J, "#params", ParamTotal);
+  printDatum(J, "#params with source location", ParamWithSrcLoc);
+  printDatum(J, "#params with type", ParamWithType);
+  printDatum(J, "#params with binary location", ParamWithLoc);
 
   // Print the summary for local variables.
-  printDatum(OS, "#local vars", LocalVarTotal);
-  printDatum(OS, "#local vars with source location", LocalVarWithSrcLoc);
-  printDatum(OS, "#local vars with type", LocalVarWithType);
-  printDatum(OS, "#local vars with binary location", LocalVarWithLoc);
+  printDatum(J, "#local vars", LocalVarTotal);
+  printDatum(J, "#local vars with source location", LocalVarWithSrcLoc);
+  printDatum(J, "#local vars with type", LocalVarWithType);
+  printDatum(J, "#local vars with binary location", LocalVarWithLoc);
 
   // Print the debug section sizes.
-  printSectionSizes(OS, Sizes);
+  printSectionSizes(J, Sizes);
 
   // Print the location statistics for variables (includes local variables
   // and formal parameters).
-  printDatum(OS, "#variables processed by location statistics",
+  printDatum(J, "#variables processed by location statistics",
              LocStats.NumVarParam);
-  printLocationStats(OS, "#variables", LocStats.VarParamLocStats);
-  printLocationStats(OS, "#variables - entry values",
+  printLocationStats(J, "#variables", LocStats.VarParamLocStats);
+  printLocationStats(J, "#variables - entry values",
                      LocStats.VarParamNonEntryValLocStats);
 
   // Print the location statistics for formal parameters.
-  printDatum(OS, "#params processed by location statistics", LocStats.NumParam);
-  printLocationStats(OS, "#params", LocStats.ParamLocStats);
-  printLocationStats(OS, "#params - entry values",
+  printDatum(J, "#params processed by location statistics", LocStats.NumParam);
+  printLocationStats(J, "#params", LocStats.ParamLocStats);
+  printLocationStats(J, "#params - entry values",
                      LocStats.ParamNonEntryValLocStats);
 
   // Print the location statistics for local variables.
-  printDatum(OS, "#local vars processed by location statistics",
+  printDatum(J, "#local vars processed by location statistics",
              LocStats.NumVar);
-  printLocationStats(OS, "#local vars", LocStats.LocalVarLocStats);
-  printLocationStats(OS, "#local vars - entry values",
+  printLocationStats(J, "#local vars", LocStats.LocalVarLocStats);
+  printLocationStats(J, "#local vars - entry values",
                      LocStats.LocalVarNonEntryValLocStats);
-  OS << "}\n";
+  J.objectEnd();
+  OS << '\n';
   LLVM_DEBUG(
       llvm::dbgs() << "Total Availability: "
                    << (int)std::round((VarParamWithLoc * 100.0) / VarParamTotal)
                    << "%\n";
       llvm::dbgs() << "PC Ranges covered: "
                    << (int)std::round((GlobalStats.ScopeBytesCovered * 100.0) /
                                       GlobalStats.ScopeBytes)
                    << "%\n");
   return true;
 }