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

Add support of JITDUMP_USE_ARCH_TIMESTAMP to perfjitdump. #111359

Merged
merged 1 commit into from
Jan 17, 2025
Merged
Changes from all commits
Commits
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
30 changes: 29 additions & 1 deletion src/coreclr/pal/src/misc/perfjitdump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@

#include "../inc/llvm/ELF.h"

#if defined(HOST_AMD64)
#include <x86intrin.h>
#endif

SET_DEFAULT_DEBUG_CHANNEL(MISC);

namespace
Expand All @@ -37,6 +41,7 @@ namespace
{
JIT_DUMP_MAGIC = 0x4A695444,
JIT_DUMP_VERSION = 1,
JITDUMP_FLAGS_ARCH_TIMESTAMP = 1 << 0,

#if defined(HOST_X86)
ELF_MACHINE = EM_386,
Expand All @@ -61,13 +66,36 @@ namespace
JIT_CODE_LOAD = 0,
};

static bool UseArchTimeStamp()
{
static bool initialized = false;
static bool useArchTimestamp = false;

if (!initialized)
{
#if defined(HOST_AMD64)
const char* archTimestamp = getenv("JITDUMP_USE_ARCH_TIMESTAMP");
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using getenv() directly you can add this to the other config settings the runtime supports:

  1. Declare a config value in configvalues.h. Here is an example that uses the 'DontPrependPrefix' flag to declare an env var without the DOTNET_ prefix:
    https://github.com/dotnet/runtime/blob/main/src/coreclr/inc/clrconfigvalues.h#L426

  2. Use CLRConfig::GetConfigValue(CLRConfig::) to retrieve the value. Example:

    fProfEnabled = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_CORECLR_ENABLE_PROFILING);

Copy link
Member

Choose a reason for hiding this comment

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

I think it would have to be CLRConfigNoCache::Get("JITDUMP_USE_ARCH_TIMESTAMP", /*noprefix*/ true, &getenv) to avoid layering issues with PAL. When this method is called with noprefix=true, it just calls getenv without anything extra.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @jkotas! - I missed the pal layering aspect in there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this mean i dont have to update the configvalue.h and simply change the getenv method with what @jkotas said.

Copy link
Member

Choose a reason for hiding this comment

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

I think calling getenv directly looks better.

useArchTimestamp = (archTimestamp != nullptr && strcmp(archTimestamp, "1") == 0);
#endif
initialized = true;
}

return useArchTimestamp;
}

static uint64_t GetTimeStampNS()
{
#if defined(HOST_AMD64)
if (UseArchTimeStamp()) {
return static_cast<uint64_t>(__rdtsc());
}
#endif
LARGE_INTEGER result;
QueryPerformanceCounter(&result);
return result.QuadPart;
}


struct FileHeader
{
FileHeader() :
Expand All @@ -78,7 +106,7 @@ namespace
pad1(0),
pid(getpid()),
timestamp(GetTimeStampNS()),
flags(0)
flags(UseArchTimeStamp() ? JITDUMP_FLAGS_ARCH_TIMESTAMP : 0)
{}

uint32_t magic;
Expand Down
Loading