<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">
Looks good. Thanks!</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">
Matthew<br>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Sam McCall <sam.mccall@gmail.com><br>
<b>Sent:</b> Tuesday, May 19, 2020 9:19 AM<br>
<b>To:</b> Voss, Matthew <Matthew.Voss@sony.com><br>
<b>Cc:</b> cfe-commits <cfe-commits@lists.llvm.org>; Sam McCall <llvmlistbot@llvm.org><br>
<b>Subject:</b> Re: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics</font>
<div> </div>
</div>
<div>
<div dir="auto">Sorry about that Matthew, I think 5bc0c8f009261425a should have fixed this (giving up on stringview entirely). It fixed the other windows based bots.
<div dir="auto"><br>
</div>
<div dir="auto">If it's no good, please do revert if you can, I'll check on it shortly too.</div>
</div>
<br>
<div class="x_gmail_quote">
<div dir="ltr" class="x_gmail_attr">On Tue, May 19, 2020, 6:14 PM Voss, Matthew <<a href="mailto:Matthew.Voss@sony.com">Matthew.Voss@sony.com</a>> wrote:<br>
</div>
<blockquote class="x_gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex">
<div dir="ltr">
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
Hi Sam,</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
<br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
This commit's tests are failing to compile on the PS4 Windows bot and our internal CI. Could you take a look? Just FYI, I may end up reverting this after an hour or so to unclog our CI.<br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
<br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
<a href="http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32433" id="x_m_5992439530538856447LPlnk452629" target="_blank" rel="noreferrer">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32433</a></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
<br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
Thanks,</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255)">
Matthew<br>
</div>
<div id="x_m_5992439530538856447appendonsend"></div>
<hr style="display:inline-block; width:98%">
<div id="x_m_5992439530538856447divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> cfe-commits <<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits-bounces@lists.llvm.org</a>>
on behalf of Sam McCall via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits@lists.llvm.org</a>><br>
<b>Sent:</b> Tuesday, May 19, 2020 4:36 AM<br>
<b>To:</b> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">
cfe-commits@lists.llvm.org</a> <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits@lists.llvm.org</a>><br>
<b>Subject:</b> [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics</font>
<div> </div>
</div>
<div><font size="2"><span style="font-size:11pt">
<div><br>
Author: Sam McCall<br>
Date: 2020-05-19T13:35:31+02:00<br>
New Revision: 9b88a190b42a03753b9c49ccea34514cb40ba4ab<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab" target="_blank" rel="noreferrer">
https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab.diff" target="_blank" rel="noreferrer">
https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab.diff</a><br>
<br>
LOG: [clangd] Add CSV export for trace metrics<br>
<br>
Summary: Example: <a href="https://docs.google.com/spreadsheets/d/1VZKGetSUTTDe9p4ooIETmdcwUod1_aE3vgD0E9x7HhI/edit" target="_blank" rel="noreferrer">
https://docs.google.com/spreadsheets/d/1VZKGetSUTTDe9p4ooIETmdcwUod1_aE3vgD0E9x7HhI/edit</a><br>
<br>
Reviewers: kadircet<br>
<br>
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb, usaxena95, cfe-commits<br>
<br>
Tags: #clang<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D79678" target="_blank" rel="noreferrer">
https://reviews.llvm.org/D79678</a><br>
<br>
Added: <br>
clang-tools-extra/clangd/test/metrics.test<br>
<br>
Modified: <br>
clang-tools-extra/clangd/support/Trace.cpp<br>
clang-tools-extra/clangd/support/Trace.h<br>
clang-tools-extra/clangd/tool/ClangdMain.cpp<br>
clang-tools-extra/clangd/unittests/support/TraceTests.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang-tools-extra/clangd/support/Trace.cpp b/clang-tools-extra/clangd/support/Trace.cpp<br>
index 6bf4816268e5..10ae461221ee 100644<br>
--- a/clang-tools-extra/clangd/support/Trace.cpp<br>
+++ b/clang-tools-extra/clangd/support/Trace.cpp<br>
@@ -189,6 +189,66 @@ class JSONTracer : public EventTracer {<br>
const llvm::sys::TimePoint<> Start;<br>
};<br>
<br>
+// We emit CSV as specified in RFC 4180: <a href="https://www.ietf.org/rfc/rfc4180.txt" target="_blank" rel="noreferrer">
https://www.ietf.org/rfc/rfc4180.txt</a>.<br>
+// \r\n line endings are used, cells with \r\n," are quoted, quotes are doubled.<br>
+class CSVMetricTracer : public EventTracer {<br>
+public:<br>
+ CSVMetricTracer(llvm::raw_ostream &Out) : Out(Out) {<br>
+ Start = std::chrono::steady_clock::now();<br>
+<br>
+ Out.SetUnbuffered(); // We write each line atomically.<br>
+ Out << "Kind,Metric,Label,Value,Timestamp\r\n";<br>
+ }<br>
+<br>
+ void record(const Metric &Metric, double Value,<br>
+ llvm::StringRef Label) override {<br>
+ assert(!needsQuote(Metric.Name));<br>
+ std::string QuotedLabel;<br>
+ if (needsQuote(Label))<br>
+ Label = QuotedLabel = quote(Label);<br>
+ uint64_t Micros = std::chrono::duration_cast<std::chrono::microseconds>(<br>
+ std::chrono::steady_clock::now() - Start)<br>
+ .count();<br>
+ std::lock_guard<std::mutex> Lock(Mu);<br>
+ Out << llvm::formatv("{0},{1},{2},{3:e},{4}.{5:6}\r\n",<br>
+ typeName(Metric.Type), Metric.Name, Label, Value,<br>
+ Micros / 1000000, Micros % 1000000);<br>
+ }<br>
+<br>
+private:<br>
+ llvm::StringRef typeName(Metric::MetricType T) {<br>
+ switch (T) {<br>
+ case Metric::Value:<br>
+ return "v";<br>
+ case Metric::Counter:<br>
+ return "c";<br>
+ case Metric::Distribution:<br>
+ return "d";<br>
+ }<br>
+ }<br>
+<br>
+ static bool needsQuote(llvm::StringRef Text) {<br>
+ // <a href="https://www.ietf.org/rfc/rfc4180.txt" target="_blank" rel="noreferrer">
https://www.ietf.org/rfc/rfc4180.txt</a> section 2.6<br>
+ return Text.find_first_of(",\"\r\n") != llvm::StringRef::npos;<br>
+ }<br>
+<br>
+ std::string quote(llvm::StringRef Text) {<br>
+ std::string Result = "\"";<br>
+ for (char C : Text) {<br>
+ Result.push_back(C);<br>
+ if (C == '"')<br>
+ Result.push_back('"');<br>
+ }<br>
+ Result.push_back('"');<br>
+ return Result;<br>
+ }<br>
+<br>
+private:<br>
+ std::mutex Mu;<br>
+ llvm::raw_ostream &Out /*GUARDED_BY(Mu)*/;<br>
+ std::chrono::steady_clock::time_point Start;<br>
+};<br>
+<br>
Key<std::unique_ptr<JSONTracer::JSONSpan>> JSONTracer::SpanKey;<br>
<br>
EventTracer *T = nullptr;<br>
@@ -206,6 +266,10 @@ std::unique_ptr<EventTracer> createJSONTracer(llvm::raw_ostream &OS,<br>
return std::make_unique<JSONTracer>(OS, Pretty);<br>
}<br>
<br>
+std::unique_ptr<EventTracer> createCSVMetricTracer(llvm::raw_ostream &OS) {<br>
+ return std::make_unique<CSVMetricTracer>(OS);<br>
+}<br>
+<br>
void log(const llvm::Twine &Message) {<br>
if (!T)<br>
return;<br>
<br>
diff --git a/clang-tools-extra/clangd/support/Trace.h b/clang-tools-extra/clangd/support/Trace.h<br>
index 90a11bb1feb4..9dc397a84b74 100644<br>
--- a/clang-tools-extra/clangd/support/Trace.h<br>
+++ b/clang-tools-extra/clangd/support/Trace.h<br>
@@ -108,11 +108,18 @@ class Session {<br>
/// Create an instance of EventTracer that produces an output in the Trace Event<br>
/// format supported by Chrome's trace viewer (chrome://tracing).<br>
///<br>
+/// FIXME: Metrics are not recorded, some could become counter events.<br>
+///<br>
/// The format is documented here:<br>
/// <a href="https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview" target="_blank" rel="noreferrer">
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview</a><br>
std::unique_ptr<EventTracer> createJSONTracer(llvm::raw_ostream &OS,<br>
bool Pretty = false);<br>
<br>
+/// Create an instance of EventTracer that outputs metric measurements as CSV.<br>
+///<br>
+/// Trace spans and instant events are ignored.<br>
+std::unique_ptr<EventTracer> createCSVMetricTracer(llvm::raw_ostream &OS);<br>
+<br>
/// Records a single instant event, associated with the current thread.<br>
void log(const llvm::Twine &Name);<br>
<br>
<br>
diff --git a/clang-tools-extra/clangd/test/metrics.test b/clang-tools-extra/clangd/test/metrics.test<br>
new file mode 100644<br>
index 000000000000..874a5dad8308<br>
--- /dev/null<br>
+++ b/clang-tools-extra/clangd/test/metrics.test<br>
@@ -0,0 +1,11 @@<br>
+# RUN: env CLANGD_METRICS=%t clangd -lit-test < %s && FileCheck %s < %t<br>
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}<br>
+---<br>
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}<br>
+# Don't verify value, timestamp, or order.<br>
+# CHECK-DAG: d,lsp_latency,textDocument/didOpen,<br>
+# CHECK-DAG: c,ast_access_diag,miss,<br>
+---<br>
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}<br>
+---<br>
+{"jsonrpc":"2.0","method":"exit"}<br>
<br>
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp<br>
index 566430167a51..e59aecb2d752 100644<br>
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp<br>
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp<br>
@@ -539,18 +539,23 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var<br>
// Setup tracing facilities if CLANGD_TRACE is set. In practice enabling a<br>
// trace flag in your editor's config is annoying, launching with<br>
// `CLANGD_TRACE=trace.json vim` is easier.<br>
- llvm::Optional<llvm::raw_fd_ostream> TraceStream;<br>
+ llvm::Optional<llvm::raw_fd_ostream> TracerStream;<br>
std::unique_ptr<trace::EventTracer> Tracer;<br>
- if (auto *TraceFile = getenv("CLANGD_TRACE")) {<br>
+ const char *JSONTraceFile = getenv("CLANGD_TRACE");<br>
+ const char *MetricsCSVFile = getenv("CLANGD_METRICS");<br>
+ const char *TracerFile = JSONTraceFile ? JSONTraceFile : MetricsCSVFile;<br>
+ if (TracerFile) {<br>
std::error_code EC;<br>
- TraceStream.emplace(TraceFile, /*ref*/ EC,<br>
- llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);<br>
+ TracerStream.emplace(TracerFile, /*ref*/ EC,<br>
+ llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);<br>
if (EC) {<br>
- TraceStream.reset();<br>
- llvm::errs() << "Error while opening trace file " << TraceFile << ": "<br>
+ TracerStream.reset();<br>
+ llvm::errs() << "Error while opening trace file " << TracerFile << ": "<br>
<< EC.message();<br>
} else {<br>
- Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);<br>
+ Tracer = (TracerFile == JSONTraceFile)<br>
+ ? trace::createJSONTracer(*TracerStream, PrettyPrint)<br>
+ : trace::createCSVMetricTracer(*TracerStream);<br>
}<br>
}<br>
<br>
<br>
diff --git a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp<br>
index 10670f79be1b..cee43f73f211 100644<br>
--- a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp<br>
+++ b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp<br>
@@ -11,6 +11,7 @@<br>
#include "support/Trace.h"<br>
#include "llvm/ADT/DenseMap.h"<br>
#include "llvm/ADT/SmallString.h"<br>
+#include "llvm/ADT/StringExtras.h"<br>
#include "llvm/ADT/StringRef.h"<br>
#include "llvm/Support/SourceMgr.h"<br>
#include "llvm/Support/Threading.h"<br>
@@ -22,7 +23,11 @@ namespace clang {<br>
namespace clangd {<br>
namespace {<br>
<br>
+using testing::_;<br>
+using testing::ElementsAre;<br>
+using testing::MatchesRegex;<br>
using testing::SizeIs;<br>
+using testing::StartsWith;<br>
<br>
MATCHER_P(StringNode, Val, "") {<br>
if (arg->getType() != llvm::yaml::Node::NK_Scalar) {<br>
@@ -138,6 +143,51 @@ TEST(MetricsTracer, LatencyTest) {<br>
EXPECT_THAT(Tracer.takeMetric(MetricName, OpName), SizeIs(1));<br>
}<br>
<br>
+class CSVMetricsTracerTest : public ::testing::Test {<br>
+protected:<br>
+ CSVMetricsTracerTest()<br>
+ : OS(Output), Tracer(trace::createCSVMetricTracer(OS)), Session(*Tracer) {<br>
+ }<br>
+ trace::Metric Dist = {"dist", trace::Metric::Distribution, "lbl"};<br>
+ trace::Metric Counter = {"cnt", trace::Metric::Counter};<br>
+<br>
+ std::vector<llvm::StringRef> outputLines() {<br>
+ // Deliberately don't flush output stream, the tracer should do that.<br>
+ // This is important when clangd crashes.<br>
+ llvm::SmallVector<llvm::StringRef, 4> Lines;<br>
+ llvm::StringRef(Output).split(Lines, "\r\n");<br>
+ return {Lines.begin(), Lines.end()};<br>
+ }<br>
+<br>
+ std::string Output;<br>
+ llvm::raw_string_ostream OS;<br>
+ std::unique_ptr<trace::EventTracer> Tracer;<br>
+ trace::Session Session;<br>
+};<br>
+<br>
+TEST_F(CSVMetricsTracerTest, RecordsValues) {<br>
+ Dist.record(1, "x");<br>
+ Counter.record(1, "");<br>
+ Dist.record(2, "y");<br>
+<br>
+ EXPECT_THAT(<br>
+ outputLines(),<br>
+ ElementsAre("Kind,Metric,Label,Value,Timestamp",<br>
+ MatchesRegex(R"(d,dist,x,1\.000000e\+00,[0-9]+\.[0-9]{6})"),<br>
+ StartsWith("c,cnt,,1.000000e+00,"),<br>
+ StartsWith("d,dist,y,2.000000e+00,"), ""));<br>
+}<br>
+<br>
+TEST_F(CSVMetricsTracerTest, Escaping) {<br>
+ Dist.record(1, ",");<br>
+ Dist.record(1, "a\"b");<br>
+ Dist.record(1, "a\nb");<br>
+<br>
+ EXPECT_THAT(outputLines(), ElementsAre(_, StartsWith(R"(d,dist,",",1)"),<br>
+ StartsWith(R"(d,dist,"a""b",1)"),<br>
+ StartsWith("d,dist,\"a\nb\",1"), ""));<br>
+}<br>
+<br>
} // namespace<br>
} // namespace clangd<br>
} // namespace clang<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" rel="noreferrer">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank" rel="noreferrer">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div>
</span></font></div>
</div>
</blockquote>
</div>
</div>
</body>
</html>