From 054d6f23813bf239a226f75dc26162ae7c57c5f4 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 23 Mar 2026 18:24:56 +0100 Subject: [PATCH 1/5] SPMI: Fixes to record-and-replay workflows Three fixes: - Skip reading compile results for superpmi replay. If the collection has compile results we never want to reuse them for superpmi replay. Various recording functions do not work properly when there already are existing compile results -- e.g. `recAllocMem` will keep the old instruction buffer instead of the new one from the replay. - Support passing a directory to -mch_files, and allow .mc files passed to -mch_files - Fix diffs report generation when a collection has zero succeeding contexts. Previously we would hit a division-by-zero error. --- src/coreclr/scripts/superpmi.py | 11 +++-- src/coreclr/tools/superpmi/mcs/verbstrip.cpp | 7 +-- .../superpmi-shared/methodcontext.cpp | 47 +++++++++++-------- .../superpmi/superpmi-shared/methodcontext.h | 12 ++--- .../superpmi-shared/methodcontextiterator.cpp | 2 +- .../superpmi-shared/methodcontextreader.cpp | 2 +- .../superpmi/superpmi/streamingsuperpmi.cpp | 2 +- .../tools/superpmi/superpmi/superpmi.cpp | 3 +- 8 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 2a1833e4361f86..38a93bd80677b1 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -2953,9 +2953,9 @@ def write_row(name, diffed_contexts, num_minopts, num_fullopts, num_missed_base, num_minopts, num_fullopts, num_missed_base, - num_missed_base / total_num_contexts * 100, + num_missed_base / total_num_contexts * 100 if total_num_contexts != 0 else 100, num_missed_diff, - num_missed_diff / total_num_contexts * 100)) + num_missed_diff / total_num_contexts * 100 if total_num_contexts != 0 else 100)) for t in rows: write_row(*t) @@ -4121,6 +4121,9 @@ def process_local_mch_files(coreclr_args, mch_files, mch_cache_dir): urls += get_files_from_path(mch_file, match_func=lambda path: any(path.lower().endswith(extension) for extension in [".mch", ".mct", ".zip"])) elif item.lower().startswith("http:") or item.lower().startswith("https:"): # probably could use urllib.parse to be more precise urls.append(item) + elif os.path.isdir(item): + # If it's a directory, we'll search recursively for .mch and .mc files in it + local_mch_files.extend(get_files_from_path(item, match_func=lambda path: any(path.lower().endswith(extension) for extension in [".mch", ".mc"]))) else: # Doesn't appear to be a UNC path (on Windows) or a URL, so just use it as-is. local_mch_files.append(item) @@ -4149,7 +4152,7 @@ def filter_local_path(path): local_mch_files += download_files(mct_urls, mch_cache_dir, fail_if_not_found=False, is_azure_storage=True, display_progress=not skip_progress) # Even though we might have downloaded MCT files, only return the set of MCH files. - local_mch_files = [file for file in local_mch_files if any(file.lower().endswith(extension) for extension in [".mch"])] + local_mch_files = [file for file in local_mch_files if any(file.lower().endswith(extension) for extension in [".mch", ".mc"])] return local_mch_files @@ -4695,7 +4698,7 @@ def get_mch_files_for_replay(local_mch_paths, filters): # If there are specified filters, only run those matching files. mch_files += get_files_from_path(item, match_func=lambda path: - any(path.endswith(extension) for extension in [".mch"]) + any(path.endswith(extension) for extension in [".mch", ".mc"]) and ((filters is None) or any(filter_item.lower() in path for filter_item in filters))) if len(mch_files) == 0: diff --git a/src/coreclr/tools/superpmi/mcs/verbstrip.cpp b/src/coreclr/tools/superpmi/mcs/verbstrip.cpp index a9fe50114c8e44..5f76e1021ffdc9 100644 --- a/src/coreclr/tools/superpmi/mcs/verbstrip.cpp +++ b/src/coreclr/tools/superpmi/mcs/verbstrip.cpp @@ -64,14 +64,9 @@ int verbStrip::DoWork( st1->Start(); } - if (!MethodContext::Initialize(loadedCount, mcb.buff, mcb.size, &mc)) + if (!MethodContext::Initialize(loadedCount, mcb.buff, mcb.size, !stripCR, &mc)) return -1; - if (stripCR) - { - delete mc->cr; - mc->cr = new CompileResult(); - } mc->saveToFile(hFileOut); savedCount++; delete mc; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 93c0e44a9f8098..ee3f676f7624f6 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -90,11 +90,14 @@ void MethodContext::Destroy() break; \ } -#define sparseReadFileCR(target, key, value) \ +#define sparseReadFileCR(target, key, value, readCompileResults) \ case PacketCR_##target: \ { \ - cr->target = new LightWeightMap(); \ - cr->target->ReadFromArray(&buff2[buffIndex], localsize); \ + if (readCompileResults) \ + { \ + cr->target = new LightWeightMap(); \ + cr->target->ReadFromArray(&buff2[buffIndex], localsize); \ + } \ break; \ } @@ -106,11 +109,14 @@ void MethodContext::Destroy() break; \ } -#define sparseReadFileCRDense(target, value) \ +#define sparseReadFileCRDense(target, value, readCompileResults) \ case PacketCR_##target: \ { \ - cr->target = new DenseLightWeightMap(); \ - cr->target->ReadFromArray(&buff2[buffIndex], localsize); \ + if (readCompileResults) \ + { \ + cr->target = new DenseLightWeightMap(); \ + cr->target->ReadFromArray(&buff2[buffIndex], localsize); \ + } \ break; \ } @@ -174,7 +180,7 @@ unsigned int MethodContext::saveToFile(HANDLE hFile) // (and sets *ppmc with new MethodContext), false on failure. // // static -bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size, /* OUT */ MethodContext** ppmc) +bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size, bool readCompileResults, /* OUT */ MethodContext** ppmc) { MethodContext* mc = new MethodContext(); mc->index = mcIndex; @@ -183,15 +189,15 @@ bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size, /* } // static -bool MethodContext::Initialize(int mcIndex, HANDLE hFile, /* OUT */ MethodContext** ppmc) +bool MethodContext::Initialize(int mcIndex, HANDLE hFile, bool readCompileResults, /* OUT */ MethodContext** ppmc) { MethodContext* mc = new MethodContext(); mc->index = mcIndex; *ppmc = mc; - return mc->Initialize(mcIndex, hFile); + return mc->Initialize(mcIndex, hFile, readCompileResults); } -bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size) +bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size, bool readCompileResults) { bool result = true; @@ -200,14 +206,15 @@ bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size) unsigned char* buff; DWORD size; MethodContext* pThis; + bool readCompileResults; } param; param.buff = buff; param.size = size; param.pThis = this; - + param.readCompileResults = readCompileResults; PAL_TRY(Param*, pParam, ¶m) { - pParam->pThis->MethodInitHelper(pParam->buff, pParam->size); + pParam->pThis->MethodInitHelper(pParam->buff, pParam->size, pParam->readCompileResults); } PAL_EXCEPT_FILTER(FilterSuperPMIExceptions_CatchMC) { @@ -219,7 +226,7 @@ bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size) return result; } -bool MethodContext::Initialize(int mcIndex, HANDLE hFile) +bool MethodContext::Initialize(int mcIndex, HANDLE hFile, bool readCompileResults) { bool result = true; @@ -227,13 +234,15 @@ bool MethodContext::Initialize(int mcIndex, HANDLE hFile) { HANDLE hFile; MethodContext* pThis; + bool readCompileResults; } param; param.hFile = hFile; param.pThis = this; + param.readCompileResults = readCompileResults; PAL_TRY(Param*, pParam, ¶m) { - pParam->pThis->MethodInitHelperFile(pParam->hFile); + pParam->pThis->MethodInitHelperFile(pParam->hFile, pParam->readCompileResults); } PAL_EXCEPT_FILTER(FilterSuperPMIExceptions_CatchMC) { @@ -245,7 +254,7 @@ bool MethodContext::Initialize(int mcIndex, HANDLE hFile) return result; } -void MethodContext::MethodInitHelperFile(HANDLE hFile) +void MethodContext::MethodInitHelperFile(HANDLE hFile, bool readCompileResults) { DWORD bytesRead; char buff[512]; @@ -258,10 +267,10 @@ void MethodContext::MethodInitHelperFile(HANDLE hFile) unsigned char* buff2 = new unsigned char[totalLen + 2]; // total + End Canary AssertCode(ReadFile(hFile, buff2, totalLen + 2, &bytesRead, NULL) == TRUE, EXCEPTIONCODE_MC); AssertCodeMsg((buff2[totalLen] == '4') && (buff2[totalLen + 1] == '2'), EXCEPTIONCODE_MC, "Didn't find end canary"); - MethodInitHelper(buff2, totalLen); + MethodInitHelper(buff2, totalLen, readCompileResults); } -void MethodContext::MethodInitHelper(unsigned char* buff2, unsigned int totalLen) +void MethodContext::MethodInitHelper(unsigned char* buff2, unsigned int totalLen, bool readCompileResults) { unsigned int buffIndex = 0; unsigned int localsize = 0; @@ -282,8 +291,8 @@ void MethodContext::MethodInitHelper(unsigned char* buff2, unsigned int totalLen #define DENSELWM(map, value) sparseReadFileDense(map, value) #include "lwmlist.h" -#define LWM(map, key, value) sparseReadFileCR(map, key, value) -#define DENSELWM(map, value) sparseReadFileCRDense(map, value) +#define LWM(map, key, value) sparseReadFileCR(map, key, value, readCompileResults) +#define DENSELWM(map, value) sparseReadFileCRDense(map, value, readCompileResults) #include "crlwmlist.h" default: diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h index 067b2154d1e52f..449a18a91caeb5 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h @@ -67,17 +67,17 @@ class MethodContext MethodContext(); private: - void MethodInitHelper(unsigned char* buff, unsigned int totalLen); - void MethodInitHelperFile(HANDLE hFile); + void MethodInitHelper(unsigned char* buff, unsigned int totalLen, bool readCompileResults); + void MethodInitHelperFile(HANDLE hFile, bool readCompileResults); - bool Initialize(int mcIndex, unsigned char* buff, DWORD size); - bool Initialize(int mcIndex, HANDLE hFile); + bool Initialize(int mcIndex, unsigned char* buff, DWORD size, bool readCompileResults); + bool Initialize(int mcIndex, HANDLE hFile, bool readCompileResults); int dumpHashToBuffer(BYTE* pBuffer, int bufLen, char* buff, int len); public: - static bool Initialize(int mcIndex, unsigned char* buff, DWORD size, /* OUT */ MethodContext** ppmc); - static bool Initialize(int mcIndex, HANDLE hFile, /* OUT */ MethodContext** ppmc); + static bool Initialize(int mcIndex, unsigned char* buff, DWORD size, bool readCompileResults, /* OUT */ MethodContext** ppmc); + static bool Initialize(int mcIndex, HANDLE hFile, bool readCompileResults, /* OUT */ MethodContext** ppmc); ~MethodContext(); void Destroy(); diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontextiterator.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontextiterator.cpp index 0f9271b142e8ee..3cd0ecbc53b2b7 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontextiterator.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontextiterator.cpp @@ -99,7 +99,7 @@ bool MethodContextIterator::MoveNext() } } - if (!MethodContext::Initialize(m_methodContextNumber, m_hFile, &m_mc)) + if (!MethodContext::Initialize(m_methodContextNumber, m_hFile, /* readCompileResults */ true, &m_mc)) return false; // If we have an array of indexes, skip the loaded indexes that have not been specified. diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontextreader.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontextreader.cpp index faa93be61fd7a3..73206fa3bedfae 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontextreader.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontextreader.cpp @@ -353,7 +353,7 @@ MethodContextBuffer MethodContextReader::GetNextMethodContextFromHash() MethodContext* mc; - if (!MethodContext::Initialize(-1, buff, mcb.size, &mc)) + if (!MethodContext::Initialize(-1, buff, mcb.size, /* readCompileResults */ true, &mc)) return MethodContextBuffer(-1); mc->dumpMethodHashToBuffer(mcHash, MM3_HASH_BUFFER_SIZE); diff --git a/src/coreclr/tools/superpmi/superpmi/streamingsuperpmi.cpp b/src/coreclr/tools/superpmi/superpmi/streamingsuperpmi.cpp index 79a08bb4f37ef5..c565d2568dfe19 100644 --- a/src/coreclr/tools/superpmi/superpmi/streamingsuperpmi.cpp +++ b/src/coreclr/tools/superpmi/superpmi/streamingsuperpmi.cpp @@ -104,7 +104,7 @@ static MethodContext* getMethodContext(int index, MethodContextReader* reader) } MethodContext* mc = nullptr; - if (!MethodContext::Initialize(index, mcb.buff, mcb.size, &mc)) + if (!MethodContext::Initialize(index, mcb.buff, mcb.size, /* readCompileResults */ false, &mc)) { return nullptr; } diff --git a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp index ded4f00e92a775..3c394dedd6e95d 100644 --- a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp +++ b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp @@ -406,10 +406,11 @@ int __cdecl main(int argc, char* argv[]) loadedCount++; const int mcIndex = reader->GetMethodContextIndex(); MethodContext* mc = nullptr; - if (!MethodContext::Initialize(mcIndex, mcb.buff, mcb.size, &mc)) + if (!MethodContext::Initialize(mcIndex, mcb.buff, mcb.size, /* readCompileResults */ false, &mc)) { return (int)SpmiResult::GeneralFailure; } + mc->Reset(); if (o.ignoreStoredConfig) { From 0287b742bdcd804174e752f2af2ebbddd0136313 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 23 Mar 2026 18:39:35 +0100 Subject: [PATCH 2/5] Unnecessary change --- src/coreclr/tools/superpmi/superpmi/superpmi.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp index 3c394dedd6e95d..c509589330c9c9 100644 --- a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp +++ b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp @@ -410,7 +410,6 @@ int __cdecl main(int argc, char* argv[]) { return (int)SpmiResult::GeneralFailure; } - mc->Reset(); if (o.ignoreStoredConfig) { From 4fe658953dda648c670c58a6e9095b6894a272bb Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 23 Mar 2026 18:43:31 +0100 Subject: [PATCH 3/5] Fix --- src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index ee3f676f7624f6..1e9fe7a1de880f 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -185,7 +185,7 @@ bool MethodContext::Initialize(int mcIndex, unsigned char* buff, DWORD size, boo MethodContext* mc = new MethodContext(); mc->index = mcIndex; *ppmc = mc; - return mc->Initialize(mcIndex, buff, size); + return mc->Initialize(mcIndex, buff, size, readCompileResults); } // static From 96bbc105c03de77c73ee33ed13bdfcbcdff8d537 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 23 Mar 2026 18:48:15 +0100 Subject: [PATCH 4/5] Feedback --- src/coreclr/scripts/superpmi.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 38a93bd80677b1..64bb14643834d7 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -2947,15 +2947,15 @@ def write_ps_row(name, diffs, perfscore_geomean): for (mch_file, base_metrics, diff_metrics, _, _, _) in asm_diffs] def write_row(name, diffed_contexts, num_minopts, num_fullopts, num_missed_base, num_missed_diff, total_num_contexts): - write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d} ({:1.2f}%)|{:,d} ({:1.2f}%)|\n".format( + write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d} ({}%)|{:,d} ({}%)|\n".format( name, diffed_contexts, num_minopts, num_fullopts, num_missed_base, - num_missed_base / total_num_contexts * 100 if total_num_contexts != 0 else 100, + "{:1.2f}".format(num_missed_base / total_num_contexts * 100) if total_num_contexts != 0 else "N/A", num_missed_diff, - num_missed_diff / total_num_contexts * 100 if total_num_contexts != 0 else 100)) + "{:1.2f}".format(num_missed_diff / total_num_contexts * 100) if total_num_contexts != 0 else "N/A")) for t in rows: write_row(*t) @@ -4698,7 +4698,7 @@ def get_mch_files_for_replay(local_mch_paths, filters): # If there are specified filters, only run those matching files. mch_files += get_files_from_path(item, match_func=lambda path: - any(path.endswith(extension) for extension in [".mch", ".mc"]) + any(path.lower().endswith(extension) for extension in [".mch", ".mc"]) and ((filters is None) or any(filter_item.lower() in path for filter_item in filters))) if len(mch_files) == 0: From 74fc5568004d5ad56d3854b02ab72f9df5ec940a Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 23 Mar 2026 18:49:35 +0100 Subject: [PATCH 5/5] Feedback --- src/coreclr/scripts/superpmi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 64bb14643834d7..bef7c9963e4455 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -4118,7 +4118,7 @@ def process_local_mch_files(coreclr_args, mch_files, mch_cache_dir): if os.path.isfile(mct_file): urls.append(mct_file) else: - urls += get_files_from_path(mch_file, match_func=lambda path: any(path.lower().endswith(extension) for extension in [".mch", ".mct", ".zip"])) + urls += get_files_from_path(mch_file, match_func=lambda path: any(path.lower().endswith(extension) for extension in [".mch", ".mc", ".mct", ".zip"])) elif item.lower().startswith("http:") or item.lower().startswith("https:"): # probably could use urllib.parse to be more precise urls.append(item) elif os.path.isdir(item): @@ -4151,7 +4151,7 @@ def filter_local_path(path): if len(mct_urls) != 0: local_mch_files += download_files(mct_urls, mch_cache_dir, fail_if_not_found=False, is_azure_storage=True, display_progress=not skip_progress) - # Even though we might have downloaded MCT files, only return the set of MCH files. + # Even though we might have downloaded MCT files, only return the set of MCH/MC files. local_mch_files = [file for file in local_mch_files if any(file.lower().endswith(extension) for extension in [".mch", ".mc"])] return local_mch_files