-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
32bit address given for 64bit builds using --prof on Windows #8221
Comments
Those shared library addresses don't look wrong to me, seems like they've simply been mapped into the lower 4 GB of the address space. I took at a quick look at the V8 code that is responsible for logging them ( |
@bnoordhuis Hm. It works properly for the 32bit build. On the 64bit build none of the |
@trevnorris Can you upload the log file somewhere? |
@bnoordhuis Ran the same script on ia32 and x64 builds. Here are the logs: https://cloudup.com/coCYebT6wT1 |
I poked around the x64 log and here is what I see:
All unknown samples fall in the range 0x7ff7bbd3a0f0-0x7ffe0cac466f which is pretty big, about 2^35, but doesn't seem to belong to any shared libraries. If you have time, can you try adding |
@bnoordhuis Same script generated 1.2GB file. It compressed down to < 50MB. Link is https://cloudup.com/iWYid8uBObd /cc @ofrobots Just want to loop you in. |
I'll take a look when I'm on a connection where downloading 50 MB doesn't take an hour. :-) |
I can't profile on Windows 10 x64 and it looks like the same thing - I really need to do some profiling at the moment! v7.10.0 Trivial test case, but this happens every time I do profiling: const add = ( a, b ) => a + b
const rand = () => Math.floor( Math.random() * 10 )
let sum = 0
for( let i = 0; i < 100000; i++ )
sum = add( sum, rand() )
console.log( sum ) isolate-0000011DAB289530-v8.log.txt
|
FYI @nodejs/platform-windows |
The original issue does not reproduce with neither v6.x nor with v8.x The issue @nrkn is having is there though. On Windows, regardless if it is x86 or x64 profiler reports 80-100% as I did find, that the issue was introduced with v8 update from #8317 /cc @nodejs/v8 |
@bzoz I've also noticed that but I don't know what causes it. The log has a lot of tick entries with nullptr samples (i.e., the PC of the sample points to address 0.) |
The profiler processor is currently showing a lot of The profiler processor expects a format that is recognized by Example line from profiler output on Linux:
Example line from profiler output on Windows:
I've added a PR that should fix the issue: #14510 |
A change to fix the issue by making the output in Windows equivalent to the ouput from other platforms has landed upstream https://chromium.googlesource.com/v8/v8/+/4229ca207e1d139d57cd9c2e72c6174d4c81878c /cc @nodejs/v8 Should this be turned into a cherry-pick floating patch while it doesn't get updated? Thanks, in advance. |
Running the profiler processor (--prof-process) with a profiler output file generated on Windows (with --prof) results in "UNKNOWN" code dominating the statistics. This is caused by the processor not correctly parsing the output of the "%p" format specifier on Windows. This commit makes the output format be the same on Windows so it can be correctly parsed by --prof-process. Original commit message: [profiler] Fix logging addresses on Windows. Change-Id: Iff0dcec95d04b85d31a452fed31b1500ad17a9f0 Reviewed-on: https://chromium-review.googlesource.com/591373 Commit-Queue: Jaroslav Sevcik <[email protected]> Reviewed-by: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#46976} Fixes: nodejs#8221
cc @nodejs/v8 (on behalf of @jaimecbernardo, this doesn't work for contributors afaik) |
Sounds like this has been resolved by moving to a higher V8 version. Feel free to re-open if I'm incorrect. |
Running
--prof
using thex64
build of node on Windows gives the following output:Notice the last line of
tick
where the address is a 64bit address. But all the addresses given to the shared libraries are only 32 bit. This results in a lot of UNKNOWN results when running--prof-process
on theisolate-*.log
file.The text was updated successfully, but these errors were encountered: