Fix and simplify inference timing logic #47711
Conversation
DilumAluthge
force-pushed
the
pc/fix-timers
branch
from
16d8353 to
d5ea1f1
Compare
src/julia.h
Outdated
| @@ -1940,6 +1940,7 @@ typedef struct _jl_task_t { | |||
| void *stkbuf; // malloc'd memory (either copybuf or stack) | |||
| size_t bufsz; // actual sizeof stkbuf | |||
| uint64_t inference_start_time; // time when inference started | |||
| unsigned int timing_inference; // whether inference is currently timing | |||
There was a problem hiding this comment.
Do we need to use another 4 bytes for this?
There was a problem hiding this comment.
If we believe that inference and codegen aren't going to exceed 65536 recursions, we can drop all of the recursion trackers to 2 bytes.
There was a problem hiding this comment.
But yes I think the issue was that I wasn't initializing the inference start time to 0, so junk times ended up being recorded. I'll adjust it to rely on the reentrant inference tracker again.
|
@DilumAluthge given that last time this failure was intermittent, how many times do we want to rerun the tests here to be sure of the fix? |
|
It was all green the first time. Let's do a second run, and if that is all green, let's go ahead and merge it and see how it does on master. |
|
Would it be possible to conceive a test which reproducibly triggers the bug, instead of hoping to hit the intermittent situation? |
|
I'm seeing a failure in Pkg, but no failure in misc. |
Yeah, this would be a good thing to add in a follow-up PR. |
|
Since #46825 is being backported to 1.9, we should also backport this fix too. |
* Fix and simplify inference timing logic * Reduce task struct size (cherry picked from commit 88a0627)
Follow-up to #46825
Should fix #47710