From ad1a41324cc9a182adef5e3d278258e508dac020 Mon Sep 17 00:00:00 2001 From: Mukul Sabharwal Date: Fri, 21 Oct 2016 15:20:02 -0700 Subject: [PATCH] Address CR feedback Commit migrated from https://github.com/dotnet/coreclr/commit/aac02c9ceb7f41ffed1353759e079f063fc953f9 --- src/coreclr/src/inc/corprof.idl | 33 +++++++++++++++++----------- src/coreclr/src/pal/prebuilt/inc/corprof.h | 30 +++++++++++++++---------- src/coreclr/src/vm/eetoprofinterfaceimpl.cpp | 24 +++++++++++++------- src/coreclr/src/vm/eetoprofinterfaceimpl.h | 8 +++++-- src/coreclr/src/vm/prestub.cpp | 4 ++-- 5 files changed, 63 insertions(+), 36 deletions(-) diff --git a/src/coreclr/src/inc/corprof.idl b/src/coreclr/src/inc/corprof.idl index b126cf2..9af1cd9 100644 --- a/src/coreclr/src/inc/corprof.idl +++ b/src/coreclr/src/inc/corprof.idl @@ -2387,18 +2387,25 @@ interface ICorProfilerCallback7 : ICorProfilerCallback6 ] interface ICorProfilerCallback8 : ICorProfilerCallback7 { - // This event is triggered whenever a metadata less method is jit compiled. + // This event is triggered whenever a dynamic method is jit compiled. // These include various IL Stubs and LCG Methods. // The goal is to provide profiler writers with enough information to identify // it to users as beyond unknown code addresses. // Note: FunctionID's provided here cannot be used to resolve to their metadata - // tokens since LCG methods have no metadata. + // tokens since dynamic methods have no metadata. // - // Documentation Note: ilHeader is only valid during the callback - - HRESULT DynamicMethodJITCompilationStarted(FunctionID functionId, LPCBYTE ilHeader); - - HRESULT DynamicMethodJITCompilationFinished(FunctionID functionId); + // Documentation Note: pILHeader is only valid during the callback + + HRESULT DynamicMethodJITCompilationStarted( + [in] FunctionID functionId, + [in] BOOL fIsSafeToBlock, + [in] LPCBYTE pILHeader, + [in] ULONG cbILHeader); + + HRESULT DynamicMethodJITCompilationFinished( + [in] FunctionID functionId, + [in] HRESULT hrStatus, + [in] BOOL fIsSafeToBlock); } @@ -3833,23 +3840,23 @@ interface ICorProfilerInfo8 : ICorProfilerInfo7 /* * Maps a managed code instruction pointer to a FunctionID. * - * GetFunctionFromIP2 fails for methods without metadata, this method works for - * both metadata and metadata-less methods and is a superset of GetFunctionFromIP2 + * GetFunctionFromIP2 fails for dynamic methods, this method works for + * both dynamic and non-dynamic methods. It is a superset of GetFunctionFromIP2 */ HRESULT GetFunctionFromIP3([in] LPCBYTE ip, [out] FunctionID *functionId, [out] ReJITID * pReJitId); /* - * Retrieves informaiton about metadata-less methods + * Retrieves informaiton about dynamic methods * - * Certain methods like IL Stubs or DynamicMethod do not have + * Certain methods like IL Stubs or LCG do not have * associated metadata that can be retrieved using the IMetaDataImport APIs. * * Such methods can be encountered by profilers through instruction pointers - * or by listening to ICorProfilerCallback::JITCompilationStartedNoMetadata + * or by listening to ICorProfilerCallback::DynamicMethodJITCompilationStarted * - * This API can be used to retrieve information about metadata less methods + * This API can be used to retrieve information about dynamic methods * including a friendly name if available. */ HRESULT GetDynamicFunctionInfo( [in] FunctionID functionId, diff --git a/src/coreclr/src/pal/prebuilt/inc/corprof.h b/src/coreclr/src/pal/prebuilt/inc/corprof.h index 56b1fbe..6778fb5 100644 --- a/src/coreclr/src/pal/prebuilt/inc/corprof.h +++ b/src/coreclr/src/pal/prebuilt/inc/corprof.h @@ -5,7 +5,7 @@ /* this ALWAYS GENERATED file contains the definitions for the interfaces */ - /* File created by MIDL compiler version 8.00.0613 */ + /* File created by MIDL compiler version 8.01.0620 */ /* @@MIDL_FILE_HEADING( ) */ #pragma warning( disable: 4049 ) /* more than 64k source lines */ @@ -5843,11 +5843,15 @@ EXTERN_C const IID IID_ICorProfilerCallback8; { public: virtual HRESULT STDMETHODCALLTYPE DynamicMethodJITCompilationStarted( - FunctionID functionId, - LPCBYTE ilHeader) = 0; + /* [in] */ FunctionID functionId, + /* [in] */ BOOL fIsSafeToBlock, + /* [in] */ LPCBYTE pILHeader, + /* [in] */ ULONG cbILHeader) = 0; virtual HRESULT STDMETHODCALLTYPE DynamicMethodJITCompilationFinished( - FunctionID functionId) = 0; + /* [in] */ FunctionID functionId, + /* [in] */ HRESULT hrStatus, + /* [in] */ BOOL fIsSafeToBlock) = 0; }; @@ -6282,12 +6286,16 @@ EXTERN_C const IID IID_ICorProfilerCallback8; HRESULT ( STDMETHODCALLTYPE *DynamicMethodJITCompilationStarted )( ICorProfilerCallback8 * This, - FunctionID functionId, - LPCBYTE ilHeader); + /* [in] */ FunctionID functionId, + /* [in] */ BOOL fIsSafeToBlock, + /* [in] */ LPCBYTE pILHeader, + /* [in] */ ULONG cbILHeader); HRESULT ( STDMETHODCALLTYPE *DynamicMethodJITCompilationFinished )( ICorProfilerCallback8 * This, - FunctionID functionId); + /* [in] */ FunctionID functionId, + /* [in] */ HRESULT hrStatus, + /* [in] */ BOOL fIsSafeToBlock); END_INTERFACE } ICorProfilerCallback8Vtbl; @@ -6586,11 +6594,11 @@ EXTERN_C const IID IID_ICorProfilerCallback8; ( (This)->lpVtbl -> ModuleInMemorySymbolsUpdated(This,moduleId) ) -#define ICorProfilerCallback8_DynamicMethodJITCompilationStarted(This,functionId,ilHeader) \ - ( (This)->lpVtbl -> DynamicMethodJITCompilationStarted(This,functionId,ilHeader) ) +#define ICorProfilerCallback8_DynamicMethodJITCompilationStarted(This,functionId,fIsSafeToBlock,pILHeader,cbILHeader) \ + ( (This)->lpVtbl -> DynamicMethodJITCompilationStarted(This,functionId,fIsSafeToBlock,pILHeader,cbILHeader) ) -#define ICorProfilerCallback8_DynamicMethodJITCompilationFinished(This,functionId) \ - ( (This)->lpVtbl -> DynamicMethodJITCompilationFinished(This,functionId) ) +#define ICorProfilerCallback8_DynamicMethodJITCompilationFinished(This,functionId,hrStatus,fIsSafeToBlock) \ + ( (This)->lpVtbl -> DynamicMethodJITCompilationFinished(This,functionId,hrStatus,fIsSafeToBlock) ) #endif /* COBJMACROS */ diff --git a/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp b/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp index 16da50e..1ceed3d 100644 --- a/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp +++ b/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp @@ -3230,7 +3230,9 @@ HRESULT EEToProfInterfaceImpl::JITCompilationStarted(FunctionID functionId, } } -HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationFinished(FunctionID functionId) +HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationFinished(FunctionID functionId, + HRESULT hrStatus, + BOOL fIsSafeToBlock) { CONTRACTL { @@ -3252,21 +3254,23 @@ HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationFinished(FunctionID fu _ASSERTE(functionId); - // Should only be called on profilers that support ICorProfilerCallback8 if (m_pCallback8 == NULL) { - return E_FAIL; + return S_OK; } { // All callbacks are really NOTHROW, but that's enforced partially by the profiler, // whose try/catch blocks aren't visible to the contract system PERMANENT_CONTRACT_VIOLATION(ThrowsViolation, ReasonProfilerCallout); - return m_pCallback8->DynamicMethodJITCompilationFinished(functionId); + return m_pCallback8->DynamicMethodJITCompilationFinished(functionId, hrStatus, fIsSafeToBlock); } } -HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationStarted(FunctionID functionId, LPCBYTE ilHeader) +HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationStarted(FunctionID functionId, + BOOL fIsSafeToBlock, + LPCBYTE pILHeader, + ULONG cbILHeader) { CONTRACTL { @@ -3288,17 +3292,21 @@ HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationStarted(FunctionID fun _ASSERTE(functionId); - // Should only be called on profilers that support ICorProfilerCallback8 + // Currently DynamicMethodJITCompilationStarted is always called with fIsSafeToBlock==TRUE. If this ever changes, + // it's safe to remove this assert, but this should serve as a trigger to change our + // public documentation to state that this callback is no longer called in preemptive mode all the time. + _ASSERTE(fIsSafeToBlock); + if (m_pCallback8 == NULL) { - return E_FAIL; + return S_OK; } { // All callbacks are really NOTHROW, but that's enforced partially by the profiler, // whose try/catch blocks aren't visible to the contract system PERMANENT_CONTRACT_VIOLATION(ThrowsViolation, ReasonProfilerCallout); - return m_pCallback8->DynamicMethodJITCompilationStarted(functionId, ilHeader); + return m_pCallback8->DynamicMethodJITCompilationStarted(functionId, fIsSafeToBlock, pILHeader, cbILHeader); } } diff --git a/src/coreclr/src/vm/eetoprofinterfaceimpl.h b/src/coreclr/src/vm/eetoprofinterfaceimpl.h index e7ec34c..76797fc 100644 --- a/src/coreclr/src/vm/eetoprofinterfaceimpl.h +++ b/src/coreclr/src/vm/eetoprofinterfaceimpl.h @@ -173,10 +173,14 @@ public: HRESULT DynamicMethodJITCompilationStarted( FunctionID functionId, - LPCBYTE ilHeader); + BOOL fIsSafeToBlock, + LPCBYTE pILHeader, + ULONG cbILHeader); HRESULT DynamicMethodJITCompilationFinished( - FunctionID functionId); + FunctionID functionId, + HRESULT hrStatus, + BOOL fIsSafeToBlock); HRESULT JITCachedFunctionSearchStarted( /* [in] */ FunctionID functionId, diff --git a/src/coreclr/src/vm/prestub.cpp b/src/coreclr/src/vm/prestub.cpp index ff897fa..572065c 100644 --- a/src/coreclr/src/vm/prestub.cpp +++ b/src/coreclr/src/vm/prestub.cpp @@ -443,7 +443,7 @@ PCODE MethodDesc::MakeJitWorker(COR_ILMETHOD_DECODER* ILHeader, DWORD flags, DWO } else { - g_profControlBlock.pProfInterface->DynamicMethodJITCompilationStarted((FunctionID) this, (LPCBYTE)ILHeader); + g_profControlBlock.pProfInterface->DynamicMethodJITCompilationStarted((FunctionID) this, TRUE, (LPCBYTE)ILHeader, ILHeader->GetSize()); } END_PIN_PROFILER(); } @@ -599,7 +599,7 @@ GotNewCode: } else { - g_profControlBlock.pProfInterface->DynamicMethodJITCompilationFinished((FunctionID) this); + g_profControlBlock.pProfInterface->DynamicMethodJITCompilationFinished((FunctionID) this, pEntry->m_hrResultCode, TRUE); } END_PIN_PROFILER(); } -- 2.7.4