Skip to content

Add repeatCount in superpmi.exe to repeat the compilation of 1 or more methods #93417

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add repeatCount in superpmi.exe to repeat the compilation of 1 or more methods #93417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 24 additions & 3 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontextreader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,15 @@ std::string MethodContextReader::CheckForPairedFile(const std::string& fileName,
}

MethodContextReader::MethodContextReader(
const char* inputFileName, const int* indexes, int indexCount, char* hash, int offset, int increment)
const char* inputFileName, const int* indexes, int indexCount, int repeatCount, char* hash, int offset, int increment)
: fileHandle(INVALID_HANDLE_VALUE)
, fileSize(0)
, curMCIndex(0)
, Indexes(indexes)
, IndexCount(indexCount)
, curIndexPos(0)
, RepeatCount(repeatCount)
, curRepeatIter(0)
, Hash(hash)
, curTOCIndex(0)
, Offset(offset)
Expand Down Expand Up @@ -130,6 +132,21 @@ MethodContextReader::~MethodContextReader()
CleanExcludedMethods();
}

void MethodContextReader::Reset()
{
this->curRepeatIter++;

// resets for `-compile`
this->curIndexPos = 0;
// resets for `-stride`
this->curMCIndex = 0;
// resets for `-methodhash`
this->curTOCIndex = 0;
// resets for `mch files`
__int64 pos = 0;
SetFilePointerEx(this->fileHandle, *(PLARGE_INTEGER)&pos, (PLARGE_INTEGER)&pos, FILE_BEGIN);
}

bool MethodContextReader::AcquireLock()
{
DWORD res = WaitForSingleObject(this->mutex, INFINITE);
Expand Down Expand Up @@ -411,13 +428,17 @@ double MethodContextReader::PercentComplete()
if (this->hasIndex() && this->hasTOC())
{
// Best estimate I can come up with...
return 100.0 * (double)this->curIndexPos / (double)this->IndexCount;
double completed = (this->curRepeatIter * this->IndexCount) + curIndexPos;
double total = this->RepeatCount * this->IndexCount;
return 100.0 * completed / total;
}
this->AcquireLock();
__int64 pos = 0;
SetFilePointerEx(this->fileHandle, *(PLARGE_INTEGER)&pos, (PLARGE_INTEGER)&pos, FILE_CURRENT);
this->ReleaseLock();
return 100.0 * (double)pos / (double)this->fileSize;
double completed = (this->curRepeatIter * (double)this->fileSize) + pos;
double total = this->RepeatCount * (double)this->fileSize;
return 100.0 * completed / total;
}

// Binary search to get this method number from the index
Expand Down
14 changes: 9 additions & 5 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontextreader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ class MethodContextReader
const int* Indexes;
int IndexCount;
int curIndexPos;
int RepeatCount;
int curRepeatIter;

// Method hash to process
// If you have an index file, these things get processed
Expand Down Expand Up @@ -124,12 +126,14 @@ class MethodContextReader

public:
MethodContextReader(const char* inputFileName,
const int* indexes = nullptr,
int indexCount = -1,
char* hash = nullptr,
int offset = -1,
int increment = -1);
const int* indexes = nullptr,
int indexCount = -1,
int repeatCount = -1,
char* hash = nullptr,
int offset = -1,
int increment = -1);
~MethodContextReader();
void Reset();

// Read a method context buffer from the ContextCollection
// (either a hive [single] or an index)
Expand Down
35 changes: 35 additions & 0 deletions src/coreclr/tools/superpmi/superpmi/commandline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ void CommandLine::DumpHelp(const char* program)
printf(" -skipCleanup\n");
printf(" Skip deletion of temporary files created by child SuperPMI processes with -parallel.\n");
printf("\n");
printf(" -repeatCount <repetition>\n");
printf(" Number of times the compilation should repeat for given method contexts. Usually used when trying to measure JIT TP for specific set of methods. Default= 1.\n");
printf("\n");
printf(" -target <target>\n");
printf(" Used by the assembly differences calculator. This specifies the target\n");
printf(" architecture for cross-compilation. Currently allowed <target> values: x64, x86, arm, arm64\n");
Expand Down Expand Up @@ -526,6 +529,38 @@ bool CommandLine::Parse(int argc, char* argv[], /* OUT */ Options* o)
{
o->skipCleanup = true;
}
else if ((_stricmp(&argv[i][1], "repeatCount") == 0))
{
if (++i < argc)
{
bool isValidCompileCount = true;
size_t nextlen = strlen(argv[i]);
for (size_t j = 0; j < nextlen; j++)
{
if (!isdigit(argv[i][j]))
{
isValidCompileCount = false;
break;
}
}
if (isValidCompileCount)
{
o->repeatCount = atoi(argv[i]);

if (o->repeatCount < 1)
{
LogError("Invalid repeat count specified, workers count must be between 1 and INT_MAX.");
DumpHelp(argv[0]);
return false;
}
}
}
else
{
DumpHelp(argv[0]);
return false;
}
}
else if ((_strnicmp(&argv[i][1], "stride", argLen) == 0))
{
// "-stride" is an internal switch used by -parallel. Usage is:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/tools/superpmi/superpmi/commandline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class CommandLine
int workerCount = -1; // Number of workers to use for /parallel mode. -1 (or 1) means don't use parallel mode.
int indexCount = -1; // If indexCount is -1 and hash points to nullptr it means compile all.
int failureLimit = -1; // Number of failures after which bail out the replay/asmdiffs.
int repeatCount = 1; // Number of times given methods should be compiled.
int* indexes = nullptr;
char* hash = nullptr;
char* methodStatsTypes = nullptr;
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,11 @@ int doParallelSuperPMI(CommandLine::Options& o)

bytesWritten += sprintf_s(cmdLine + bytesWritten, MAX_CMDLINE_SIZE - bytesWritten, " -v ewmin %s", spmiArgs);

if (o.repeatCount > 1)
{
bytesWritten += sprintf_s(cmdLine + bytesWritten, MAX_CMDLINE_SIZE - bytesWritten, " -repeatCount %d", o.repeatCount);
}

SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(sa);
sa.lpSecurityDescriptor = NULL;
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/tools/superpmi/superpmi/superpmi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ int __cdecl main(int argc, char* argv[])
// The method context reader handles skipping any unrequested method contexts
// Used in conjunction with an MCI file, it does a lot less work...
MethodContextReader* reader =
new MethodContextReader(o.nameOfInputMethodContextFile, o.indexes, o.indexCount, o.hash, o.offset, o.increment);
new MethodContextReader(o.nameOfInputMethodContextFile, o.indexes, o.indexCount, o.repeatCount, o.hash, o.offset, o.increment);
if (!reader->isValid())
{
return (int)SpmiResult::GeneralFailure;
Expand Down Expand Up @@ -327,6 +327,8 @@ int __cdecl main(int argc, char* argv[])
}
}

for (int iter = 0; iter < o.repeatCount; iter++)
{
Comment on lines +330 to +331
Copy link
Member

Choose a reason for hiding this comment

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

It seems like we shouldn't need to change the MethodContextReader at all. Instead of the "repeatCount" loop here, couldn't it be just before the CompileResult* crl = mc->cr; line? (There might need to be some kind of saving/restoring of the originalCR).

Copy link
Member Author

@kunalspathak kunalspathak Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

possibly, the problem with that though is that the display of percent completion report will be slower. E.g. if someone pass -repeatcount 10000 for entire mch, the time after which you would see the console output would be after 10000 * 500 compilations.

Copy link
Member

Choose a reason for hiding this comment

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

It seems wrong that the MethodContextReader should have any knowledge of the repeatCount.

How about have the repeatCount loop include the %complete logging, and displayed % complete would be repeatIteration * reader->PercentComplete() / repeatCount. For a repeatCount of 1, this would be no change.

Then, change it to output every 500 iterations whether that is due to loadedCount or repeatCount, e.g.,

if ((totalIterations++ % 500 == 0) && (loadedCount > 0))

Copy link
Member

Choose a reason for hiding this comment

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

How about have the repeatCount loop include the %complete logging, and displayed % complete would be repeatIteration * reader->PercentComplete() / repeatCount. For a repeatCount of 1, this would be no change.

My math here is wrong. It would need to be:

100.0 * (((reader->CurrentCount() - 1) * repeatCount + repeatIteration) / (repeatCount * reader->TotalCount())

for newly exposed MethodContextReader curIndexPos (as CurrentCount()) and IndexCount (as TotalCount())

(this depends on curIndexPos returning 1 when you're processing the first method context. curIndexPos and IndexCount also depend on there being a .MCT file, which there isn't always, in which case we would have to return the pos/fileSize like PercentComplete does, which wouldn't be as accurate.)

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry, just seeing your comment. One of the other problem of moving loop to iterate over jit->CompileMethod() is that it will be a tight loop without showing the % progress unless we add another LogVerbose inside it. So for eg, if I pass -c 59 -repeatCount 10000, i am expected to see % completion after every 500 iterations or so. Unless I am misunderstanding your suggestion of where to add the logging.

while (true)
{
MethodContextBuffer mcb = reader->GetNextMethodContext();
Expand Down Expand Up @@ -709,6 +711,8 @@ int __cdecl main(int argc, char* argv[])

delete crl;
delete mc;
}
reader->Reset();
}
delete reader;

Expand Down