Uh oh!
There was an error while loading. Please reload this page.
[NativeAOT] Enable async runtime suspension and return hijacking on unix-arm64 - #73216
Conversation
VSadov
commented
Aug 4, 2022
/azp run runtime-extra-platforms |
VSadov
commented
Aug 4, 2022
I think this is ready to review/merge. |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| void* pExceptionRecord, void* pContextRecord); | ||
| int32_t __stdcall RhpVectoredExceptionHandler(PEXCEPTION_POINTERS pExPtrs); | ||
| // REVIEW: this is no longer used by pInvokes and use in hijack seems bogus. Remove? |
There was a problem hiding this comment.
This was meant to protect against SEH exceptions entering the managed code and leaving it in inconsistent state. We should re-introduce this protection. Could you please create an issue on it and link it from here?
If PInvoke throws SEH exception, it is going unwind the managed portion of the stack and the runtime is going to be left in inconsistent state currently.
There was a problem hiding this comment.
Makes sense, I will log an issue.
But we do not need this for hijack probes, right? There is no scenario that I can think of where we expect exceptions from waiting for GC.
There was a problem hiding this comment.
It may be there so that fatal crashes like stackoverflow or access violations caused by process state corruptions terminate the process immediately instead letting it to propagate.
| // ldp with pre/post/no offset | ||
| // x010 100x x1xx xxxx xxxx xxxx xxxx xxxx | ||
| #define LDP_BITS2 0x28400000 |
There was a problem hiding this comment.
I will trust you that you got these magic numbers right :-)
There was a problem hiding this comment.
It took quite some time to construct and proof-read these.
The "BITS" parts for the individual instruction patterns ended up matching constants from instrarm64.h
And that is encouraging :-)
runtime/src/coreclr/jit/instrsarm64.h
Line 613 in 2201016
| // we could be catching threads in restartable sequences such as LL/SC style interlocked on ARM64 | ||
| // and forcing them to restart. | ||
| // if interrupt mechanism is fast, eagerness could be hurting our overall progress. | ||
| waitCycles += 10000; |
There was a problem hiding this comment.
Should we really keep this growing without any limit?
There was a problem hiding this comment.
Ultimately the wait is unbounded, since suspension cannot gracefully fail. - "It is done, when it is done".
Practically, we can have very good timings here.
Both waiting too long and waiting not enough between re-hijacking can make the whole thing last longer.
Here I observed that we would interrupt a thread doing LL/SC InterlockedSomething loop, interrupting would invalidate its monitor, so SC would fail and start over - just in time for us to interrupt it again - leading to very long hangs.
Fixed spin counts are often a bad idea as the guesstimate may be wrong when running on a different platform. I just made the spin count to adjust in a naive way for now.
Dealing with this loop is the next/last part in the NativeAOT suspension work item. #67805
I plan to make this similar to what CoreCLR does:
- interrupt threads and hijack/suspend accordingly
- spin-check without re-hijacking as we are making progress - the common case is everything suspends quickly.
- otherwise wait for progress with a 1 msec timeout, and if timed out, try hijacking again - to deal with remaining strugglers.
VSadov
commented
Aug 5, 2022
Thanks!! |
On ARM64 Linux. See if #73216 helped with the hang.
On ARM64 Linux. See if #73216 helped with the hang.
Contributes to: #67805