WIP_Update to Windows Profiler #20687
Conversation
Added high precision sleep Applied random factor between 0 and 2 to timeout
Moved StartingTime to not include rand()
src/signals-win.c
Outdated
| TIMECAPS tc; | ||
| if (MMSYSERR_NOERROR!=timeGetDevCaps(&tc, sizeof(tc))) { | ||
| fputs("failed to get timer resolution",stderr); | ||
| hBtThread = 0; | ||
| return 0; | ||
| } | ||
|
|
||
| //Precision timer |
There was a problem hiding this comment.
Please use multiples of 4 spaces for indentation rather than tabs for consistency with the rest of the repository.
There was a problem hiding this comment.
Sorry about that. I've recommited.
src/signals-win.c
Outdated
| QueryPerformanceCounter(&StartingTime); | ||
| int time_elapsed = 0; | ||
| while (time_elapsed ==0) | ||
| { |
There was a problem hiding this comment.
There's trailing whitespace on this line, which is what's causing the check to fail
|
If we're randomizing the profile interval, we should probably do that across all platforms. Could we split the randomization from the windows-specific timer change into separate PR's? |
src/signals-win.c
Outdated
| //calculate start time for high precision sleep | ||
| QueryPerformanceCounter(&StartingTime); | ||
| int time_elapsed = 0; | ||
| while (time_elapsed ==0) |
There was a problem hiding this comment.
very minor, but would look a little nicer with spaces on both sides of the ==, not just on the left
src/signals-win.c
Outdated
| } | ||
| else | ||
| { | ||
| //if time has not elasped offer up processor to another thread |
|
for reference/reviewer selection, looks like this code was originally authored in 8247ac7 |
|
is this still WIP, or RFC? |
|
It's still WIP. Small profiler intervals can cause julia to crash (see below). I haven't had a chance to chase it down, but could this be due to the StackWalk64 not being thread-safe as mentioned in #2597 @vtjnash Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks. |
|
There's still interest in #9224, but we should do it on all platforms. We don't want to busy wait (like this PR), since that can skew the results even more significantly however. |
#9224
I've made two changes to the Profiler on Windows.
@timholy