Make Hostx64/arm crossgen /CreatePerfMap behave the same as Hostarm/arm crossgen #20035
Conversation
|
@jkotas Can you please review this? |
src/vm/perfmap.cpp
Outdated
| @@ -173,7 +179,7 @@ void PerfMap::LogMethod(MethodDesc * pMethod, PCODE pCode, size_t codeSize) | |||
| // Build the map file line. | |||
| StackScratchBuffer scratch; | |||
| SString line; | |||
| line.Printf("%p %x %s\n", pCode, codeSize, fullMethodSignature.GetANSI(scratch)); | |||
| line.Printf(FMT_TARGET_ADDR " %x %s\n", pCode, codeSize, fullMethodSignature.GetANSI(scratch)); | |||
There was a problem hiding this comment.
These are 32-bit offsets in crossgen generated map files, not actual addresses. It took me a while to figure this out.
@brianrob Would it make sense to always print these as 32-bit numbers in crossgen generated map files?
There was a problem hiding this comment.
pCode can either be an offset in the case where a native image perfmap is being built or an actual address in the case where we're building a perfmap for a live process. In the latter, case PerfMap::LogMethod is called by PerfMap::LogJITCompiledMethod which takes the actual code address via pCode. So, this does need to support writing the full width of a pointer.
There was a problem hiding this comment.
Right, this needs to be a full pointer for the JITed case. My question is whether it needs to be full 64-bit pointer in the native image case as well. We seem to be just wasting space on zeros.
There was a problem hiding this comment.
Ah. As long as we can guarantee that the value will fit into 32-bits then it should be fine. I'm not 100% sure about this as the values that we calculate this from are of type TADDR. If you know it to be true that the values fit into 32-bits then that should be fine for the crossgen case. I could also imagine us changing the format string (if possible) to just not write the full width of the value with prepended zeros and instead just write the minimal number of characters. Then we get the savings on the JITed case too.
There was a problem hiding this comment.
According to https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jit-interface.txt
perf supports a simple JIT interface to resolve symbols for dynamic code generated
by a JIT.The JIT has to write a /tmp/perf-%d.map (%d = pid of process) file
This is a text file.
Each line has the following format, fields separated with spaces:
START SIZE symbolname
START and SIZE are hex numbers without 0x.
symbolname is the rest of the line, so it could contain special characters.The ownership of the file has to match the process.
it should be safe to remove leading zeros from format string
src/vm/perfmap.cpp
Outdated
| @@ -12,6 +12,12 @@ | |||
| #include "perfinfo.h" | |||
| #include "pal.h" | |||
|
|
|||
| #ifdef _TARGET_64BIT_ | |||
There was a problem hiding this comment.
This works, but it will make them file less readable. I would keep the crossgen addresses/offsets padded to 32-bit. Something like:
#ifdef CROSSGEN_COMPILE
// The code addresses are actually native image offsets during crossgen. Print them as 32-bit numbers for consistent output when cross-targeting and to make the output more compact.
#define FMT_TARGET_ADDR "%08x"
#else
#define FMT_TARGET_ADDR "%p"
#endif
|
Rebased/squashed into three separate commits |
This PR addresses couple issues with
/CreatePerfMapwith Hostx64/arm32 crossgen:line.Printfuses host-specific "%p" -> change to target-specific format stringFMT_TARGET_ADDRin src/vm/perfmap.cppAcquireImageusessizeof(PVOID)-> replace withTARGET_POINTER_SIZEin src/vm/readytoruninfo.cpppModule->HasNativeImage() || pModule->IsReadyToRun()withpModule->HasNativeOrReadyToRunImage()in src/vm/compile.cpp