From bb57c259e66d28d5149d3a2f1aeb6345705c0add Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 20 Apr 2023 01:37:31 -0700 Subject: [PATCH 1/3] Write perfmap and jitdump files to /tmp by default --- src/coreclr/inc/clrconfigvalues.h | 2 +- src/coreclr/vm/perfinfo.cpp | 10 ++----- src/coreclr/vm/perfinfo.h | 2 +- src/coreclr/vm/perfmap.cpp | 50 +++++++++++++++++++++++-------- src/coreclr/vm/perfmap.h | 2 +- 5 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index d06900034778ae..7f6698684f9fbf 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -526,7 +526,7 @@ RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_ProfAPI_ValidateNGENInstrumentation, W("Pro #ifdef FEATURE_PERFMAP RETAIL_CONFIG_DWORD_INFO(EXTERNAL_PerfMapEnabled, W("PerfMapEnabled"), 0, "This flag is used on Linux to enable writing /tmp/perf-$pid.map. It is disabled by default") -RETAIL_CONFIG_STRING_INFO(EXTERNAL_PerfMapJitDumpPath, W("PerfMapJitDumpPath"), "Specifies a path to write the perf jitdump file. Defaults to GetTempPathA()") +RETAIL_CONFIG_STRING_INFO(EXTERNAL_PerfMapJitDumpPath, W("PerfMapJitDumpPath"), "Specifies a path to write the perf jitdump file. Defaults to /tmp") RETAIL_CONFIG_DWORD_INFO(EXTERNAL_PerfMapIgnoreSignal, W("PerfMapIgnoreSignal"), 0, "When perf map is enabled, this option will configure the specified signal to be accepted and ignored as a marker in the perf logs. It is disabled by default") RETAIL_CONFIG_DWORD_INFO(EXTERNAL_PerfMapShowOptimizationTiers, W("PerfMapShowOptimizationTiers"), 1, "Shows optimization tiers in the perf map for methods, as part of the symbol name. Useful for seeing separate stack frames for different optimization tiers of each method.") RETAIL_CONFIG_STRING_INFO(EXTERNAL_NativeImagePerfMapFormat, W("NativeImagePerfMapFormat"), "Specifies the format of native image perfmap files generated by crossgen. Valid options are RVA or OFFSET.") diff --git a/src/coreclr/vm/perfinfo.cpp b/src/coreclr/vm/perfinfo.cpp index 869476ea0fd574..83c63eeacf4db0 100644 --- a/src/coreclr/vm/perfinfo.cpp +++ b/src/coreclr/vm/perfinfo.cpp @@ -10,19 +10,13 @@ #include "perfinfo.h" #include "pal.h" -PerfInfo::PerfInfo(int pid) +PerfInfo::PerfInfo(int pid, const char* basePath) : m_Stream(nullptr) { LIMITED_METHOD_CONTRACT; - SString tempPath; - if (!WszGetTempPath(tempPath)) - { - return; - } - SString path; - path.Printf("%Sperfinfo-%d.map", tempPath.GetUnicode(), pid); + path.Printf("%sperfinfo-%d.map", basePath, pid); OpenFile(path); } diff --git a/src/coreclr/vm/perfinfo.h b/src/coreclr/vm/perfinfo.h index 759a844f30925c..79305a03b6f2cb 100644 --- a/src/coreclr/vm/perfinfo.h +++ b/src/coreclr/vm/perfinfo.h @@ -20,7 +20,7 @@ */ class PerfInfo { public: - PerfInfo(int pid); + PerfInfo(int pid, const char* basePath); ~PerfInfo(); void LogImage(PEFile* pFile, WCHAR* guid); diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 5eda731ec8bd0f..0a724ba56aa77d 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -22,6 +22,14 @@ #define FMT_CODE_ADDR "%p" #endif +#ifndef __ANDROID__ +#define TEMP_DIRECTORY_PATH "/tmp/" +#else +// On Android, "/tmp/" doesn't exist; temporary files should go to +// /data/local/tmp/ +#define TEMP_DIRECTORY_PATH "/data/local/tmp/" +#endif + Volatile PerfMap::s_enabled = false; PerfMap * PerfMap::s_Current = nullptr; bool PerfMap::s_ShowOptimizationTiers = false; @@ -43,11 +51,28 @@ void PerfMap::Initialize() // Only enable the map if requested. if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapEnabled) == ALL || CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapEnabled) == PERFMAP) { + char perfmapPath[4096]; + + // CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapJitDumpPath) returns a LPWSTR + // Use GetEnvironmentVariableA because it is simpler. + // Keep comment here to make it searchable. + DWORD len = GetEnvironmentVariableA("COMPlus_PerfMapJitDumpPath", perfmapPath, sizeof(perfmapPath) - 1); + + if (len == 0) + { + len = GetEnvironmentVariableA("DOTNET_PerfMapJitDumpPath", perfmapPath, sizeof(perfmapPath) - 1); + + if (len == 0) + { + strcpy_s(perfmapPath, sizeof(perfmapPath), TEMP_DIRECTORY_PATH); + } + } + // Get the current process id. int currentPid = GetCurrentProcessId(); // Create the map. - s_Current = new PerfMap(currentPid); + s_Current = new PerfMap(currentPid, perfmapPath); int signalNum = (int) CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapIgnoreSignal); @@ -75,7 +100,12 @@ void PerfMap::Initialize() if (len == 0) { - GetTempPathA(sizeof(jitdumpPath) - 1, jitdumpPath); + len = GetEnvironmentVariableA("DOTNET_PerfMapJitDumpPath", jitdumpPath, sizeof(jitdumpPath) - 1); + + if (len == 0) + { + strcpy_s(jitdumpPath, sizeof(jitdumpPath), TEMP_DIRECTORY_PATH); + } } PAL_PerfJitDump_Start(jitdumpPath); @@ -103,7 +133,7 @@ void PerfMap::Destroy() } // Construct a new map for the process. -PerfMap::PerfMap(int pid) +PerfMap::PerfMap(int pid, const char* path) { LIMITED_METHOD_CONTRACT; @@ -111,19 +141,13 @@ PerfMap::PerfMap(int pid) m_ErrorEncountered = false; // Build the path to the map file on disk. - WCHAR tempPath[MAX_LONGPATH+1]; - if(!GetTempPathW(MAX_LONGPATH, tempPath)) - { - return; - } - - SString path; - path.Printf("%Sperf-%d.map", &tempPath, pid); + SString pathFile; + pathFile.Printf("%sperf-%d.map", path, pid); // Open the map file for writing. - OpenFile(path); + OpenFile(pathFile); - m_PerfInfo = new PerfInfo(pid); + m_PerfInfo = new PerfInfo(pid, path); } // Construct a new map without a specified file name. diff --git a/src/coreclr/vm/perfmap.h b/src/coreclr/vm/perfmap.h index 636ac81830aa66..d42d421c4af6f4 100644 --- a/src/coreclr/vm/perfmap.h +++ b/src/coreclr/vm/perfmap.h @@ -31,7 +31,7 @@ class PerfMap bool m_ErrorEncountered; // Construct a new map for the specified pid. - PerfMap(int pid); + PerfMap(int pid, const char* path); protected: // Indicates whether optimization tiers should be shown for methods in perf maps From 1a29631c591c1b3f80286967483bf9a0ba074c1b Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Mon, 10 Jul 2023 11:09:15 -0400 Subject: [PATCH 2/3] Simplify perfmap path generation --- src/coreclr/vm/perfinfo.cpp | 2 +- src/coreclr/vm/perfmap.cpp | 91 ++++++++++++++++--------------------- src/coreclr/vm/perfmap.h | 3 ++ 3 files changed, 44 insertions(+), 52 deletions(-) diff --git a/src/coreclr/vm/perfinfo.cpp b/src/coreclr/vm/perfinfo.cpp index 83c63eeacf4db0..d3a7fc0238e54f 100644 --- a/src/coreclr/vm/perfinfo.cpp +++ b/src/coreclr/vm/perfinfo.cpp @@ -16,7 +16,7 @@ PerfInfo::PerfInfo(int pid, const char* basePath) LIMITED_METHOD_CONTRACT; SString path; - path.Printf("%sperfinfo-%d.map", basePath, pid); + path.Printf("%s/perfinfo-%d.map", basePath, pid); OpenFile(path); } diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 0a724ba56aa77d..e121646d2d7b92 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -23,11 +23,11 @@ #endif #ifndef __ANDROID__ -#define TEMP_DIRECTORY_PATH "/tmp/" +#define TEMP_DIRECTORY_PATH "/tmp" #else // On Android, "/tmp/" doesn't exist; temporary files should go to // /data/local/tmp/ -#define TEMP_DIRECTORY_PATH "/data/local/tmp/" +#define TEMP_DIRECTORY_PATH "/data/local/tmp" #endif Volatile PerfMap::s_enabled = false; @@ -48,31 +48,24 @@ void PerfMap::Initialize() { LIMITED_METHOD_CONTRACT; - // Only enable the map if requested. - if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapEnabled) == ALL || CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapEnabled) == PERFMAP) + const DWORD perfMapEnabled = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapEnabled); + if (perfMapEnabled == DISABLED) { - char perfmapPath[4096]; - - // CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapJitDumpPath) returns a LPWSTR - // Use GetEnvironmentVariableA because it is simpler. - // Keep comment here to make it searchable. - DWORD len = GetEnvironmentVariableA("COMPlus_PerfMapJitDumpPath", perfmapPath, sizeof(perfmapPath) - 1); - - if (len == 0) - { - len = GetEnvironmentVariableA("DOTNET_PerfMapJitDumpPath", perfmapPath, sizeof(perfmapPath) - 1); + return; + } - if (len == 0) - { - strcpy_s(perfmapPath, sizeof(perfmapPath), TEMP_DIRECTORY_PATH); - } - } + // Build the path to the map file on disk. + char tempPathBuffer[MAX_LONGPATH+1]; + const char* tempPath = InternalConstructPath(tempPathBuffer, sizeof(tempPathBuffer)); + // Only enable the map if requested. + if (perfMapEnabled == ALL || perfMapEnabled == PERFMAP) + { // Get the current process id. int currentPid = GetCurrentProcessId(); // Create the map. - s_Current = new PerfMap(currentPid, perfmapPath); + s_Current = new PerfMap(currentPid, tempPath); int signalNum = (int) CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapIgnoreSignal); @@ -80,43 +73,39 @@ void PerfMap::Initialize() { PAL_IgnoreProfileSignal(signalNum); } - - if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapShowOptimizationTiers) != 0) - { - s_ShowOptimizationTiers = true; - } - - s_enabled = true; } - if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapEnabled) == ALL || CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapEnabled) == JITDUMP) + // only enable JitDumps if requested + if (perfMapEnabled == ALL || perfMapEnabled == JITDUMP) { - char jitdumpPath[4096]; - - // CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapJitDumpPath) returns a LPWSTR - // Use GetEnvironmentVariableA because it is simpler. - // Keep comment here to make it searchable. - DWORD len = GetEnvironmentVariableA("COMPlus_PerfMapJitDumpPath", jitdumpPath, sizeof(jitdumpPath) - 1); - - if (len == 0) - { - len = GetEnvironmentVariableA("DOTNET_PerfMapJitDumpPath", jitdumpPath, sizeof(jitdumpPath) - 1); + PAL_PerfJitDump_Start(tempPath); + } - if (len == 0) - { - strcpy_s(jitdumpPath, sizeof(jitdumpPath), TEMP_DIRECTORY_PATH); - } - } + if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapShowOptimizationTiers) != 0) + { + s_ShowOptimizationTiers = true; + } + + s_enabled = true; +} - PAL_PerfJitDump_Start(jitdumpPath); +// InternalConstructPath is guaranteed to return a non-null path with the path separator appended to the end +// the function uses the input buffer only whe PerfMapJitDumpPath environment variable is set +const char * PerfMap::InternalConstructPath(char *tmpBuf, int lenBuf) +{ + DWORD len = GetEnvironmentVariableA("DOTNET_PerfMapJitDumpPath", tmpBuf, lenBuf); + if (len == 0) + { + len = GetEnvironmentVariableA("COMPlus_PerfMapJitDumpPath", tmpBuf, lenBuf); + } - if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapShowOptimizationTiers) != 0) - { - s_ShowOptimizationTiers = true; - } - - s_enabled = true; + if (len == 0 || // GetEnvironmentVariableA returns 0 if the variable is not found, + len >= lenBuf) // or the length of the string not including the null terminator on success. + { + return TEMP_DIRECTORY_PATH; } + + return tmpBuf; } // Destroy the map for the process - called from EEShutdownHelper. @@ -142,7 +131,7 @@ PerfMap::PerfMap(int pid, const char* path) // Build the path to the map file on disk. SString pathFile; - pathFile.Printf("%sperf-%d.map", path, pid); + pathFile.Printf("%s/perf-%d.map", path, pid); // Open the map file for writing. OpenFile(pathFile); diff --git a/src/coreclr/vm/perfmap.h b/src/coreclr/vm/perfmap.h index d42d421c4af6f4..f9665f8b27f0c2 100644 --- a/src/coreclr/vm/perfmap.h +++ b/src/coreclr/vm/perfmap.h @@ -33,6 +33,9 @@ class PerfMap // Construct a new map for the specified pid. PerfMap(int pid, const char* path); + // Default to /tmp or use DOTNET_PerfMapJitDumpPath if set + static const char* InternalConstructPath(char *tmpBuf, int lenBuf); + protected: // Indicates whether optimization tiers should be shown for methods in perf maps static bool s_ShowOptimizationTiers; From 4971630bbcc0b4bf2c5d0a6d355330de00686977 Mon Sep 17 00:00:00 2001 From: Tom McDonald Date: Thu, 13 Jul 2023 13:19:25 -0400 Subject: [PATCH 3/3] Minor comment update --- src/coreclr/vm/perfmap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index e121646d2d7b92..88bdd42dbe0279 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -89,7 +89,7 @@ void PerfMap::Initialize() s_enabled = true; } -// InternalConstructPath is guaranteed to return a non-null path with the path separator appended to the end +// InternalConstructPath is guaranteed to return a non-null path // the function uses the input buffer only whe PerfMapJitDumpPath environment variable is set const char * PerfMap::InternalConstructPath(char *tmpBuf, int lenBuf) {