Conversation
vm/linking.cpp
Outdated
| #if defined(KE_LINUX) | ||
| // These have to be shared among all environments, as the filenames are only distinguished by PID. | ||
| // Once we support multiple environments per process we'll need some internal locking. | ||
| static std::unique_ptr<PerfJitFile> perf_jit_file = {}; |
There was a problem hiding this comment.
It'd be better to find a globally visible place to put these, but for now this is fine. You shouldn't need the initializer though.
There was a problem hiding this comment.
I can't find a good spot elsewhere for these - any suggestions? I don't think we have any naturally global objects (Environments are the top-level instance), so they'd need to be global vars or static members of something (probably Environment?)
vm/linking.cpp
Outdated
| // These have to be shared among all environments, as the filenames are only distinguished by PID. | ||
| // Once we support multiple environments per process we'll need some internal locking. | ||
| static std::unique_ptr<PerfJitFile> perf_jit_file; | ||
| static std::unique_ptr<PerfJitdumpFile> perf_jitdump_file; |
There was a problem hiding this comment.
It's fine here. My thinking re: moving toward Environment was, that's all code we know has to be audited for a multi-threaded refactoring. But squirreled away here, no one is going to find it.
So if we keep it here, can we put a static std::mutex in and hold the lock while writing the perf data? Then we know it's at least future-proofed.
There was a problem hiding this comment.
I moved the implementation out of linking and moved ownership of the files to Environment, I think that is looking a lot cleaner now indeed.
dvander
left a comment
dvander
left a comment
There was a problem hiding this comment.
Thanks, this is indeed much nicer.
| } | ||
| } | ||
|
|
||
| if (!pRuntime->Name()) |
There was a problem hiding this comment.
This thing bugs me still, they're very different checks.
There was a problem hiding this comment.
The old one was broken by previous refactoring elsewhere, the new one is the intended behaviour.
4 participants
Implemented here is the original perf JIT format which is widely supported but only supports simple name mapping, and the newer "jitdump" format which additionally supports embedding the machine code and source mapping information (along with a bunch of other things we don't currently have in the JIT).
Example usage:
Using the jitdump source mapping will probably require compiling perf from source, due to this bug, but it works very well.