Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Enhance metrics reported by superpmi diffs #74584

Merged
merged 25 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
706953d
Add some artificial diffs
jakobbotsch Aug 25, 2022
ba7bd0c
JIT: Enhance metrics reported by superpmi diffs
jakobbotsch Aug 25, 2022
4ad92a1
Fix PAL build
jakobbotsch Aug 25, 2022
13a4ab7
Get some diffs in release
jakobbotsch Aug 25, 2022
75f71a0
Remove percentage, try some color
jakobbotsch Aug 25, 2022
631d489
Various adjustments
jakobbotsch Aug 29, 2022
c8d4b8e
Reorder/rename some columns
jakobbotsch Sep 2, 2022
5d79fb9
Separate sections for overall/minopts/fullopts
jakobbotsch Sep 5, 2022
70c4abd
Merge branch 'main' of github.com:dotnet/runtime into spmi-metrics
jakobbotsch Sep 5, 2022
e7087df
Fixes
jakobbotsch Sep 5, 2022
286a1f5
Merge branch 'spmi-metrics' of github.com:jakobbotsch/runtime into sp…
jakobbotsch Sep 5, 2022
654a5d9
Optimized -> FullOpts, switch some wording, switch back to bytes
jakobbotsch Sep 5, 2022
365a0d4
Right align numbers
jakobbotsch Sep 5, 2022
38a85fb
Change some headings
jakobbotsch Sep 9, 2022
366523a
Handle JIT switching to minopts/fullopts
jakobbotsch Sep 13, 2022
e58e404
Revert forward sub change
jakobbotsch Sep 13, 2022
17778ad
Leave sections unexpanded by default
jakobbotsch Sep 14, 2022
fa45a99
Address feedback
jakobbotsch Sep 14, 2022
a710c45
Revert "Revert forward sub change"
jakobbotsch Sep 14, 2022
e0cc967
Add a horizontal line separator, heading for jit-analyze output
jakobbotsch Sep 14, 2022
3fe712a
Address feedback
jakobbotsch Sep 15, 2022
d180d77
Merge branch 'main' of github.com:dotnet/runtime into spmi-metrics
jakobbotsch Sep 15, 2022
1a23870
Revert "Revert "Revert forward sub change""
jakobbotsch Sep 19, 2022
6910d3c
Random comment change to trigger SPMI without diffs
jakobbotsch Sep 19, 2022
25285d5
Hide "missed" contexts line when there are none
jakobbotsch Sep 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 170 additions & 51 deletions src/coreclr/scripts/superpmi.py

Large diffs are not rendered by default.

35 changes: 2 additions & 33 deletions src/coreclr/scripts/superpmi_diffs_summarize.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,39 +89,8 @@ def append_diff_file(f, arch, file_name, full_file_path, asmdiffs):
else:
diffs_found = True
f.write("## {} {}\n".format(diff_os, diff_arch))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me why this section is removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sections are printed by superpmi.py now.

if asmdiffs:
# Contents of asmdiffs are large so create a
# <summary><details> ... </details> disclosure section around
# the file and additionally provide some instructions.
f.write("""\

<details>

<summary>{0} {1} details</summary>

Summary file: `{2}`

To reproduce these diffs on Windows {3}:
```
superpmi.py asmdiffs -target_os {0} -target_arch {1} -arch {3}
```

""".format(diff_os, diff_arch, file_name, arch))

# Now write the contents
f.write(contents)

# Write the footer (close the <details> section)
f.write("""\

</details>

""")

else:
f.write(contents)
f.write("\n")
f.write(contents)
f.write("\n")

return diffs_found

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,7 @@ DWORD MethodContext::repGetJitFlags(CORJIT_FLAGS* jitFlags, DWORD sizeInBytes)
DEBUG_REP(dmpGetJitFlags(0, value));

CORJIT_FLAGS* resultFlags = (CORJIT_FLAGS*)GetJitFlags->GetBuffer(value.A);
Assert(sizeInBytes >= value.B);
memcpy(jitFlags, resultFlags, value.B);
InitReadyToRunFlag(resultFlags);
return value.B;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/tools/superpmi/superpmi-shared/standardpch.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,6 @@ static inline void __debugbreak()
}
#endif

#include <minipal/utils.h>

#endif // STANDARDPCH_H
4 changes: 0 additions & 4 deletions src/coreclr/tools/superpmi/superpmi/commandline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,6 @@ void CommandLine::DumpHelp(const char* program)
printf("\n");
printf(" -metricsSummary <file name>, -baseMetricsSummary <file name.csv>\n");
printf(" Emit a summary of metrics to the specified file\n");
printf(" Currently includes:\n");
printf(" Total number of successful SPMI compiles\n");
printf(" Total number of failing SPMI compiles\n");
printf(" Total amount of ASM code in bytes\n");
printf("\n");
printf(" -diffMetricsSummary <file name>\n");
printf(" Same as above, but emit for the diff/second JIT");
Expand Down
26 changes: 24 additions & 2 deletions src/coreclr/tools/superpmi/superpmi/jitinstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ extern "C" DLLEXPORT NOINLINE void Instrumentor_GetInsCount(UINT64* result)
}
}

JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, MetricsSummary* metrics)
JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, MetricsSummary* metrics, bool* isMinOpts)
{
struct Param : FilterSuperPMIExceptionsParam_CaptureException
{
Expand All @@ -309,13 +309,17 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i
int mcIndex;
bool collectThroughput;
MetricsSummary* metrics;
bool* isMinOpts;
} param;
param.pThis = this;
param.result = RESULT_SUCCESS; // assume success
param.flags = 0;
param.mcIndex = mcIndex;
param.collectThroughput = collectThroughput;
param.metrics = metrics;
param.isMinOpts = isMinOpts;

*isMinOpts = false;

// store to instance field our raw values, so we can figure things out a bit later...
mc = MethodToCompile;
Expand All @@ -335,6 +339,14 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i
CORINFO_OS os = CORINFO_WINNT;

pParam->pThis->mc->repCompileMethod(&pParam->info, &pParam->flags, &os);
CORJIT_FLAGS jitFlags;
pParam->pThis->mc->repGetJitFlags(&jitFlags, sizeof(jitFlags));

*pParam->isMinOpts =
jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_DEBUG_CODE) ||
jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MIN_OPT) ||
jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_TIER0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think Tier0 code is same as MinOpt.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today they are practically the same, I think. They both lead to OptimizarionDisabled() returning true.

Copy link
Member Author

@jakobbotsch jakobbotsch Sep 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Also note that Compiler::Options::MinOpts() returns true in tier-0 compilations, so it's not wrong to say that tier-0 compilations are MinOpts)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I was checking yesterday and comparing it with "debuggable code" which is different. I was thinking that "debuggable code" is min-opts.


if (pParam->collectThroughput)
{
pParam->pThis->lt.Start();
Expand All @@ -347,6 +359,17 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i
pParam->pThis->lt.Stop();
pParam->pThis->times[0] = pParam->pThis->lt.GetCycles();
}

CorInfoMethodRuntimeFlags flags = pParam->pThis->mc->cr->repSetMethodAttribs(pParam->info.ftn);
if ((flags & CORINFO_FLG_SWITCHED_TO_MIN_OPT) != 0)
{
*pParam->isMinOpts = true;
}
else if ((flags & CORINFO_FLG_SWITCHED_TO_OPTIMIZED) != 0)
{
*pParam->isMinOpts = false;
}

if (jitResult == CORJIT_SKIPPED)
{
SPMI_TARGET_ARCHITECTURE targetArch = GetSpmiTargetArchitecture();
Expand Down Expand Up @@ -436,7 +459,6 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i
{
metrics->SuccessfulCompiles++;
metrics->NumExecutedInstructions += static_cast<long long>(insCountAfter - insCountBefore);

}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/superpmi/superpmi/jitinstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class JitInstance

bool resetConfig(MethodContext* firstContext);

Result CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, class MetricsSummary* summary);
Result CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, struct MetricsSummary* metrics, bool* isMinOpts);

const WCHAR* getForceOption(const WCHAR* key);
const WCHAR* getOption(const WCHAR* key);
Expand Down
173 changes: 121 additions & 52 deletions src/coreclr/tools/superpmi/superpmi/metricssummary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,23 @@
#include "metricssummary.h"
#include "logging.h"

struct HandleCloser
void MetricsSummary::AggregateFrom(const MetricsSummary& other)
{
void operator()(HANDLE hFile)
{
CloseHandle(hFile);
}
};
SuccessfulCompiles += other.SuccessfulCompiles;
FailingCompiles += other.FailingCompiles;
MissingCompiles += other.MissingCompiles;
NumCodeBytes += other.NumCodeBytes;
NumDiffedCodeBytes += other.NumDiffedCodeBytes;
NumExecutedInstructions += other.NumExecutedInstructions;
NumDiffExecutedInstructions += other.NumDiffExecutedInstructions;
}

void MetricsSummaries::AggregateFrom(const MetricsSummaries& other)
{
Overall.AggregateFrom(other.Overall);
MinOpts.AggregateFrom(other.MinOpts);
FullOpts.AggregateFrom(other.FullOpts);
}

struct FileHandleWrapper
{
Expand All @@ -31,37 +41,61 @@ struct FileHandleWrapper
HANDLE hFile;
};

bool MetricsSummary::SaveToFile(const char* path)
static bool FilePrintf(HANDLE hFile, const char* fmt, ...)
{
va_list args;
va_start(args, fmt);

char buffer[4096];
int len = vsprintf_s(buffer, ARRAY_SIZE(buffer), fmt, args);
DWORD numWritten;
bool result =
WriteFile(hFile, buffer, static_cast<DWORD>(len), &numWritten, nullptr) && (numWritten == static_cast<DWORD>(len));

va_end(args);

return result;
}

bool MetricsSummaries::SaveToFile(const char* path)
{
FileHandleWrapper file(CreateFile(path, GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr));
if (file.get() == INVALID_HANDLE_VALUE)
{
return false;
}

char buffer[4096];
int len =
sprintf_s(
buffer, sizeof(buffer),
"Successful compiles,Failing compiles,Missing compiles,Code bytes,Diffed code bytes,Executed instructions,Diff executed instructions\n"
"%d,%d,%d,%lld,%lld,%lld,%lld\n",
SuccessfulCompiles,
FailingCompiles,
MissingCompiles,
NumCodeBytes,
NumDiffedCodeBytes,
NumExecutedInstructions,
NumDiffExecutedInstructions);
DWORD numWritten;
if (!WriteFile(file.get(), buffer, static_cast<DWORD>(len), &numWritten, nullptr) || numWritten != static_cast<DWORD>(len))
if (!FilePrintf(
file.get(),
"Successful compiles,Failing compiles,Missing compiles,"
"Code bytes,Diffed code bytes,Executed instructions,Diff executed instructions,Name\n"))
{
return false;
}

return true;
return
WriteRow(file.get(), "Overall", Overall) &&
WriteRow(file.get(), "MinOpts", MinOpts) &&
WriteRow(file.get(), "FullOpts", FullOpts);
}

bool MetricsSummaries::WriteRow(HANDLE hFile, const char* name, const MetricsSummary& summary)
{
return
FilePrintf(
hFile,
"%d,%d,%d,%lld,%lld,%lld,%lld,%s\n",
summary.SuccessfulCompiles,
summary.FailingCompiles,
summary.MissingCompiles,
summary.NumCodeBytes,
summary.NumDiffedCodeBytes,
summary.NumExecutedInstructions,
summary.NumDiffExecutedInstructions,
name);
}

bool MetricsSummary::LoadFromFile(const char* path, MetricsSummary* metrics)
bool MetricsSummaries::LoadFromFile(const char* path, MetricsSummaries* metrics)
{
FileHandleWrapper file(CreateFile(path, GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr));
if (file.get() == INVALID_HANDLE_VALUE)
Expand All @@ -76,38 +110,73 @@ bool MetricsSummary::LoadFromFile(const char* path, MetricsSummary* metrics)
}

DWORD stringLen = static_cast<DWORD>(len.QuadPart);
std::vector<char> content(stringLen + 1);
std::vector<char> content(stringLen);
DWORD numRead;
if (!ReadFile(file.get(), content.data(), stringLen, &numRead, nullptr) || numRead != stringLen)
if (!ReadFile(file.get(), content.data(), stringLen, &numRead, nullptr) || (numRead != stringLen))
{
return false;
}

content[stringLen] = '\0';

int scanResult =
sscanf_s(
content.data(),
"Successful compiles,Failing compiles,Missing compiles,Code bytes,Diffed code bytes,Executed instructions,Diff executed instructions\n"
"%d,%d,%d,%lld,%lld,%lld,%lld\n",
&metrics->SuccessfulCompiles,
&metrics->FailingCompiles,
&metrics->MissingCompiles,
&metrics->NumCodeBytes,
&metrics->NumDiffedCodeBytes,
&metrics->NumExecutedInstructions,
&metrics->NumDiffExecutedInstructions);

return scanResult == 7;
}
std::vector<char> line;
size_t index = 0;
auto nextLine = [&line, &content, &index]()
{
size_t end = index;
while ((end < content.size()) && (content[end] != '\r') && (content[end] != '\n'))
{
end++;
}

void MetricsSummary::AggregateFrom(const MetricsSummary& other)
{
SuccessfulCompiles += other.SuccessfulCompiles;
FailingCompiles += other.FailingCompiles;
MissingCompiles += other.MissingCompiles;
NumCodeBytes += other.NumCodeBytes;
NumDiffedCodeBytes += other.NumDiffedCodeBytes;
NumExecutedInstructions += other.NumExecutedInstructions;
NumDiffExecutedInstructions += other.NumDiffExecutedInstructions;
line.resize(end - index + 1);
memcpy(line.data(), &content[index], end - index);
line[end - index] = '\0';

index = end;
if ((index < content.size()) && (content[index] == '\r'))
index++;
if ((index < content.size()) && (content[index] == '\n'))
index++;
};

*metrics = MetricsSummaries();
nextLine();
bool result = true;
while (index < content.size())
{
nextLine();
MetricsSummary summary;

char name[32];
int scanResult =
sscanf_s(
line.data(),
"%d,%d,%d,%lld,%lld,%lld,%lld,%s",
&summary.SuccessfulCompiles,
&summary.FailingCompiles,
&summary.MissingCompiles,
&summary.NumCodeBytes,
&summary.NumDiffedCodeBytes,
&summary.NumExecutedInstructions,
&summary.NumDiffExecutedInstructions,
name, (unsigned)sizeof(name));

if (scanResult == 8)
{
MetricsSummary* tarSummary = nullptr;
if (strcmp(name, "Overall") == 0)
metrics->Overall = summary;
else if (strcmp(name, "MinOpts") == 0)
metrics->MinOpts = summary;
else if (strcmp(name, "FullOpts") == 0)
metrics->FullOpts = summary;
else
result = false;
}
else
{
result = false;
}
}

return result;
}
20 changes: 16 additions & 4 deletions src/coreclr/tools/superpmi/superpmi/metricssummary.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
#ifndef _MetricsSummary
#define _MetricsSummary

class MetricsSummary
struct MetricsSummary
{
public:
// Number of methods successfully jitted.
int SuccessfulCompiles = 0;
// Number of methods that failed jitting.
Expand All @@ -23,9 +22,22 @@ class MetricsSummary
// Number of executed instructions inside contexts that were successfully diffed.
long long NumDiffExecutedInstructions = 0;

bool SaveToFile(const char* path);
static bool LoadFromFile(const char* path, MetricsSummary* metrics);
void AggregateFrom(const MetricsSummary& other);
};

class MetricsSummaries
{
public:
MetricsSummary Overall;
MetricsSummary MinOpts;
MetricsSummary FullOpts;

void AggregateFrom(const MetricsSummaries& other);

bool SaveToFile(const char* path);
static bool LoadFromFile(const char* path, MetricsSummaries* metrics);
private:
static bool WriteRow(HANDLE hFile, const char* name, const MetricsSummary& summary);
};

#endif
Loading