Skip to content

Commit

Permalink
Improve windows hardware exception handling performance (#74426)
Browse files Browse the repository at this point in the history
* Improve windows hardware exception handling performance

With my recent change that modified hardware exception handling so that
the related managed exception is thrown directly from the vectored
exception handler, the performance of handling such exceptions have
regressed. Several exception handling dotnet/performance microbenchmarks
have regressed upto 15%.
The reason for the regression was the larger number of stack frames
between the exception raising and the actual handler frame. With a
recent change that @AntonLapounov has made to fix process corrupting
exceptions handling, the regression went down to 8%. This change moves
the location where we raise the exception down to the
ClrVectoredExceptionHandlerShim, which means to the closest possible
frame to the managed code.
This gets rid of the regression completely.
  • Loading branch information
janvorli authored Aug 24, 2022
1 parent 4b14c92 commit 8641256
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 58 deletions.
96 changes: 42 additions & 54 deletions src/coreclr/vm/excep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6723,27 +6723,16 @@ bool ShouldHandleManagedFault(

#ifndef TARGET_UNIX

LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo);

enum VEH_ACTION
{
VEH_NO_ACTION = 0,
VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION,
VEH_CONTINUE_EXECUTION,
VEH_CONTINUE_SEARCH,
VEH_EXECUTE_HANDLER
};


VEH_ACTION WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo);
VEH_ACTION WINAPI CLRVectoredExceptionHandlerPhase3(PEXCEPTION_POINTERS pExceptionInfo);

LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo)
VEH_ACTION WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo)
{
// It is not safe to execute code inside VM after we shutdown EE. One example is DisablePreemptiveGC
// will block forever.
if (g_fForbidEnterEE)
{
return EXCEPTION_CONTINUE_SEARCH;
return VEH_CONTINUE_SEARCH;
}


Expand Down Expand Up @@ -6833,7 +6822,7 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo)
pExceptionInfo->ContextRecord->Rip = hijackArgs.ReturnAddress;
}

return EXCEPTION_CONTINUE_EXECUTION;
return VEH_CONTINUE_EXECUTION;
}
#endif

Expand All @@ -6857,11 +6846,9 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo)
//
// Not an Out-of-memory situation, so no need for a forbid fault region here
//
return EXCEPTION_CONTINUE_SEARCH;
return VEH_CONTINUE_SEARCH;
}

LONG retVal = 0;

// We can't probe here, because we won't return from the CLRVectoredExceptionHandlerPhase2
// on WIN64
//
Expand All @@ -6872,15 +6859,10 @@ LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo)
CantAllocHolder caHolder;
}

retVal = CLRVectoredExceptionHandlerPhase2(pExceptionInfo);

//
//END_ENTRYPOINT_VOIDRET;
//
return retVal;
return CLRVectoredExceptionHandlerPhase2(pExceptionInfo);
}

LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo)
VEH_ACTION WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo)
{
//
// DO NOT USE CONTRACTS HERE AS THIS ROUTINE MAY NEVER RETURN. You can use
Expand Down Expand Up @@ -6914,27 +6896,16 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo
action = CLRVectoredExceptionHandlerPhase3(pExceptionInfo);
}

if (action == VEH_CONTINUE_EXECUTION)
if ((action == VEH_CONTINUE_EXECUTION) || (action == VEH_CONTINUE_SEARCH) || (action == VEH_EXECUTE_HANDLER))
{
return EXCEPTION_CONTINUE_EXECUTION;
}

if (action == VEH_CONTINUE_SEARCH)
{
return EXCEPTION_CONTINUE_SEARCH;
}

if (action == VEH_EXECUTE_HANDLER)
{
return EXCEPTION_EXECUTE_HANDLER;
return action;
}

#if defined(FEATURE_EH_FUNCLETS)

if (action == VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION)
{
HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord);
return EXCEPTION_CONTINUE_EXECUTION;
return action;
}

#endif // defined(FEATURE_EH_FUNCLETS)
Expand All @@ -6954,7 +6925,7 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo
// the choice to break the no-trigger region after taking all necessary precautions.
if (IsDebuggerFault(pExceptionRecord, pExceptionInfo->ContextRecord, pExceptionRecord->ExceptionCode, GetThreadNULLOk()))
{
return EXCEPTION_CONTINUE_EXECUTION;
return VEH_CONTINUE_EXECUTION;
}
}

Expand Down Expand Up @@ -6986,11 +6957,11 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo
{
// The breakpoint was not ours. Someone else can handle it. (Or if not, we'll get it again as
// an unhandled exception.)
return EXCEPTION_CONTINUE_SEARCH;
return VEH_CONTINUE_SEARCH;
}

// The breakpoint was from managed or the runtime. Handle it.
return UserBreakpointFilter(pExceptionInfo);
return (VEH_ACTION)UserBreakpointFilter(pExceptionInfo);
}

#if defined(FEATURE_EH_FUNCLETS)
Expand All @@ -7008,15 +6979,11 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo

if (fShouldHandleManagedFault)
{
//
// HandleManagedFault may never return, so we cannot use a forbid fault region around it.
//
HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord);
return EXCEPTION_CONTINUE_EXECUTION;
}
return VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION;
}
#endif // defined(FEATURE_EH_FUNCLETS)

return EXCEPTION_EXECUTE_HANDLER;
return VEH_EXECUTE_HANDLER;
}

/*
Expand Down Expand Up @@ -7583,13 +7550,34 @@ LONG WINAPI CLRVectoredExceptionHandlerShim(PEXCEPTION_POINTERS pExceptionInfo)
if (pThread || fExceptionInEE)
{
if (!bIsGCMarker)
result = CLRVectoredExceptionHandler(pExceptionInfo);
else
result = EXCEPTION_CONTINUE_EXECUTION;
{
VEH_ACTION action = CLRVectoredExceptionHandler(pExceptionInfo);

#ifdef FEATURE_EH_FUNCLETS
if (VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION == action)
{
//
// HandleManagedFault may never return, so we cannot use a forbid fault region around it.
//
HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord);
return EXCEPTION_CONTINUE_EXECUTION;
}
#endif // FEATURE_EH_FUNCLETS

if (EXCEPTION_EXECUTE_HANDLER == result)
if (VEH_EXECUTE_HANDLER == action)
{
result = EXCEPTION_CONTINUE_SEARCH;
}
else
{
_ASSERTE((action == VEH_CONTINUE_EXECUTION) || (action == VEH_CONTINUE_SEARCH));
result = (LONG)action;
}

}
else
{
result = EXCEPTION_CONTINUE_SEARCH;
result = EXCEPTION_CONTINUE_EXECUTION;
}

#ifdef _DEBUG
Expand Down
11 changes: 10 additions & 1 deletion src/coreclr/vm/exceptmacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,16 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowOM();

#endif // !defined(FEATURE_EH_FUNCLETS)

LONG WINAPI CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo);
enum VEH_ACTION
{
VEH_NO_ACTION = -3,
VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION = -2,
VEH_CONTINUE_EXECUTION = EXCEPTION_CONTINUE_EXECUTION,
VEH_CONTINUE_SEARCH = EXCEPTION_CONTINUE_SEARCH,
VEH_EXECUTE_HANDLER = EXCEPTION_EXECUTE_HANDLER
};

VEH_ACTION CLRVectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo);

// Actual UEF worker prototype for use by GCUnhandledExceptionFilter.
extern LONG InternalUnhandledExceptionFilter_Worker(PEXCEPTION_POINTERS pExceptionInfo);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/i386/excepx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1255,13 +1255,13 @@ CPFH_FirstPassHandler(EXCEPTION_RECORD *pExceptionRecord,
// Call to the vectored handler to give other parts of the Runtime a chance to jump in and take over an
// exception before we do too much with it. The most important point in the vectored handler is not to toggle
// the GC mode.
DWORD filter = CLRVectoredExceptionHandler(&ptrs);
VEH_ACTION filter = CLRVectoredExceptionHandler(&ptrs);

if (filter == (DWORD) EXCEPTION_CONTINUE_EXECUTION)
if (filter == VEH_CONTINUE_EXECUTION)
{
return ExceptionContinueExecution;
}
else if (filter == EXCEPTION_CONTINUE_SEARCH)
else if (filter == VEH_CONTINUE_SEARCH)
{
return ExceptionContinueSearch;
}
Expand Down

0 comments on commit 8641256

Please sign in to comment.