Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Removing m_hostAssemblyMap and ceremony around it #61292

Merged
merged 3 commits into from
Nov 8, 2021
Merged

Removing m_hostAssemblyMap and ceremony around it #61292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43bfdd7
remove m_hostAssemblyMap and ceremony around it
VSadov Nov 7, 2021
3e4e39e
Removes SHash::AddPhases - support for callouts in hosted scenarios.
VSadov Nov 7, 2021
a23468f
typos
VSadov Nov 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/coreclr/binder/assembly.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace BINDER_SPACE
m_pAssemblyName = NULL;
m_isInTPA = false;
m_pBinder = NULL;
m_domainAssembly = NULL;
}

Assembly::~Assembly()
Expand Down
14 changes: 14 additions & 0 deletions src/coreclr/binder/inc/assembly.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

#include "bundle.h"

class DomainAssembly;

namespace BINDER_SPACE
{
// BINDER_SPACE::Assembly represents a result of binding to an actual assembly (PEImage)
Expand All @@ -53,12 +55,24 @@ namespace BINDER_SPACE
return m_pBinder;
}

DomainAssembly* GetDomainAssembly()
{
return m_domainAssembly;
}

void SetDomainAssembly(DomainAssembly* value)
{
_ASSERTE(value == NULL || m_domainAssembly == NULL);
m_domainAssembly = value;
}

private:
LONG m_cRef;
PEImage *m_pPEImage;
AssemblyName *m_pAssemblyName;
AssemblyBinder *m_pBinder;
bool m_isInTPA;
DomainAssembly *m_domainAssembly;

inline void SetBinder(AssemblyBinder *pBinder)
{
Expand Down
9 changes: 1 addition & 8 deletions src/coreclr/inc/CrstTypes.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,6 @@ Crst HandleTable
SameLevelAs HandleTable
End

Crst HostAssemblyMap
End

Crst HostAssemblyMapAdd
AcquiredBefore HostAssemblyMap
End

Crst IbcProfile
End

Expand Down Expand Up @@ -586,4 +579,4 @@ End

Crst PgoData
AcquiredBefore LoaderHeap
End
End
163 changes: 78 additions & 85 deletions src/coreclr/inc/crsttypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
//

#ifndef __CRST_TYPES_INCLUDED
Expand All @@ -11,7 +10,7 @@

// This file describes the range of Crst types available and their mapping to a numeric level (used by the
// runtime in debug mode to validate we're deadlock free). To modify these settings edit the
// file:CrstTypes.def file and run the clr\bin\CrstTypeTool utility to generate a new version of this file.
// file:CrstTypes.def file and run the clr\artifacts\CrstTypeTool utility to generate a new version of this file.

// Each Crst type is declared as a value in the following CrstType enum.
enum CrstType
Expand Down Expand Up @@ -59,85 +58,83 @@ enum CrstType
CrstGCCover = 40,
CrstGlobalStrLiteralMap = 41,
CrstHandleTable = 42,
CrstHostAssemblyMap = 43,
CrstHostAssemblyMapAdd = 44,
CrstIbcProfile = 45,
CrstIJWFixupData = 46,
CrstIJWHash = 47,
CrstILStubGen = 48,
CrstInlineTrackingMap = 49,
CrstInstMethodHashTable = 50,
CrstInterop = 51,
CrstInteropData = 52,
CrstIsJMCMethod = 53,
CrstISymUnmanagedReader = 54,
CrstJit = 55,
CrstJitGenericHandleCache = 56,
CrstJitInlineTrackingMap = 57,
CrstJitPatchpoint = 58,
CrstJitPerf = 59,
CrstJumpStubCache = 60,
CrstLeafLock = 61,
CrstListLock = 62,
CrstLoaderAllocator = 63,
CrstLoaderAllocatorReferences = 64,
CrstLoaderHeap = 65,
CrstManagedObjectWrapperMap = 66,
CrstMethodDescBackpatchInfoTracker = 67,
CrstModule = 68,
CrstModuleFixup = 69,
CrstModuleLookupTable = 70,
CrstMulticoreJitHash = 71,
CrstMulticoreJitManager = 72,
CrstNativeImageEagerFixups = 73,
CrstNativeImageLoad = 74,
CrstNls = 75,
CrstNotifyGdb = 76,
CrstObjectList = 77,
CrstPEImage = 78,
CrstPendingTypeLoadEntry = 79,
CrstPgoData = 80,
CrstPinnedByrefValidation = 81,
CrstProfilerGCRefDataFreeList = 82,
CrstProfilingAPIStatus = 83,
CrstRCWCache = 84,
CrstRCWCleanupList = 85,
CrstReadyToRunEntryPointToMethodDescMap = 86,
CrstReflection = 87,
CrstReJITGlobalRequest = 88,
CrstRetThunkCache = 89,
CrstSavedExceptionInfo = 90,
CrstSaveModuleProfileData = 91,
CrstSecurityStackwalkCache = 92,
CrstSigConvert = 93,
CrstSingleUseLock = 94,
CrstSpecialStatics = 95,
CrstStackSampler = 96,
CrstStressLog = 97,
CrstStubCache = 98,
CrstStubDispatchCache = 99,
CrstStubUnwindInfoHeapSegments = 100,
CrstSyncBlockCache = 101,
CrstSyncHashLock = 102,
CrstSystemBaseDomain = 103,
CrstSystemDomain = 104,
CrstSystemDomainDelayedUnloadList = 105,
CrstThreadIdDispenser = 106,
CrstThreadpoolTimerQueue = 107,
CrstThreadpoolWaitThreads = 108,
CrstThreadpoolWorker = 109,
CrstThreadStore = 110,
CrstTieredCompilation = 111,
CrstTypeEquivalenceMap = 112,
CrstTypeIDMap = 113,
CrstUMEntryThunkCache = 114,
CrstUMEntryThunkFreeListLock = 115,
CrstUniqueStack = 116,
CrstUnresolvedClassLock = 117,
CrstUnwindInfoTableLock = 118,
CrstVSDIndirectionCellLock = 119,
CrstWrapperTemplate = 120,
kNumberOfCrstTypes = 121
CrstIbcProfile = 43,
CrstIJWFixupData = 44,
CrstIJWHash = 45,
CrstILStubGen = 46,
CrstInlineTrackingMap = 47,
CrstInstMethodHashTable = 48,
CrstInterop = 49,
CrstInteropData = 50,
CrstIsJMCMethod = 51,
CrstISymUnmanagedReader = 52,
CrstJit = 53,
CrstJitGenericHandleCache = 54,
CrstJitInlineTrackingMap = 55,
CrstJitPatchpoint = 56,
CrstJitPerf = 57,
CrstJumpStubCache = 58,
CrstLeafLock = 59,
CrstListLock = 60,
CrstLoaderAllocator = 61,
CrstLoaderAllocatorReferences = 62,
CrstLoaderHeap = 63,
CrstManagedObjectWrapperMap = 64,
CrstMethodDescBackpatchInfoTracker = 65,
CrstModule = 66,
CrstModuleFixup = 67,
CrstModuleLookupTable = 68,
CrstMulticoreJitHash = 69,
CrstMulticoreJitManager = 70,
CrstNativeImageEagerFixups = 71,
CrstNativeImageLoad = 72,
CrstNls = 73,
CrstNotifyGdb = 74,
CrstObjectList = 75,
CrstPEImage = 76,
CrstPendingTypeLoadEntry = 77,
CrstPgoData = 78,
CrstPinnedByrefValidation = 79,
CrstProfilerGCRefDataFreeList = 80,
CrstProfilingAPIStatus = 81,
CrstRCWCache = 82,
CrstRCWCleanupList = 83,
CrstReadyToRunEntryPointToMethodDescMap = 84,
CrstReflection = 85,
CrstReJITGlobalRequest = 86,
CrstRetThunkCache = 87,
CrstSavedExceptionInfo = 88,
CrstSaveModuleProfileData = 89,
CrstSecurityStackwalkCache = 90,
CrstSigConvert = 91,
CrstSingleUseLock = 92,
CrstSpecialStatics = 93,
CrstStackSampler = 94,
CrstStressLog = 95,
CrstStubCache = 96,
CrstStubDispatchCache = 97,
CrstStubUnwindInfoHeapSegments = 98,
CrstSyncBlockCache = 99,
CrstSyncHashLock = 100,
CrstSystemBaseDomain = 101,
CrstSystemDomain = 102,
CrstSystemDomainDelayedUnloadList = 103,
CrstThreadIdDispenser = 104,
CrstThreadpoolTimerQueue = 105,
CrstThreadpoolWaitThreads = 106,
CrstThreadpoolWorker = 107,
CrstThreadStore = 108,
CrstTieredCompilation = 109,
CrstTypeEquivalenceMap = 110,
CrstTypeIDMap = 111,
CrstUMEntryThunkCache = 112,
CrstUMEntryThunkFreeListLock = 113,
CrstUniqueStack = 114,
CrstUnresolvedClassLock = 115,
CrstUnwindInfoTableLock = 116,
CrstVSDIndirectionCellLock = 117,
CrstWrapperTemplate = 118,
kNumberOfCrstTypes = 119
};

#endif // __CRST_TYPES_INCLUDED
Expand Down Expand Up @@ -191,8 +188,6 @@ int g_rgCrstLevelMap[] =
10, // CrstGCCover
13, // CrstGlobalStrLiteralMap
1, // CrstHandleTable
0, // CrstHostAssemblyMap
3, // CrstHostAssemblyMapAdd
0, // CrstIbcProfile
8, // CrstIJWFixupData
0, // CrstIJWHash
Expand Down Expand Up @@ -317,8 +312,6 @@ LPCSTR g_rgCrstNameMap[] =
"CrstGCCover",
"CrstGlobalStrLiteralMap",
"CrstHandleTable",
"CrstHostAssemblyMap",
"CrstHostAssemblyMapAdd",
"CrstIbcProfile",
"CrstIJWFixupData",
"CrstIJWHash",
Expand Down
97 changes: 3 additions & 94 deletions src/coreclr/inc/shash.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,6 @@ class EMPTY_BASES_DECL SHash : public TRAITS
// NoThrow version of CheckGrowth function. Returns FALSE on failure.
BOOL CheckGrowthNoThrow();

// See if it is OK to grow the hash table by one element. If not, allocate new
// hash table and return it together with its size *pcNewSize (used by code:AddPhases).
// Returns NULL if there already is space for one element.
element_t * CheckGrowth_OnlyAllocateNewTable(count_t * pcNewSize);

// Allocates new resized hash table for growth. Does not update the hash table on the object.
// The new size is computed based on the current population, growth factor, and maximum density factor.
element_t * Grow_OnlyAllocateNewTable(count_t * pcNewSize);
Expand All @@ -278,7 +273,7 @@ class EMPTY_BASES_DECL SHash : public TRAITS

// Utility function to allocate new table (does not copy the values into it yet). Returns the size of new table in
// *pcNewTableSize (finds next prime).
// Phase 1 of code:Reallocate - it is split to support code:AddPhases.
// Phase 1 of code:Reallocate
element_t * AllocateNewTable(count_t requestedSize, count_t * pcNewTableSize);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there still any good reason for Reallocate to be split into 3 methods?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not small/trivial methods and have different responsibilities, so I figured it is ok to leave them as separate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also other callers - like AllocateNewTable is also called by Grow_OnlyAllocateNewTable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Others have multiple callers too.


// NoThrow version of AllocateNewTable utility function. Returns NULL on failure.
Expand All @@ -287,11 +282,11 @@ class EMPTY_BASES_DECL SHash : public TRAITS
// Utility function to replace old table with newly allocated table (as allocated by
// code:AllocateNewTable). Copies all 'old' values into the new table first.
// Returns the old table. Caller is expected to delete it (via code:DeleteOldTable).
// Phase 2 of code:Reallocate - it is split to support code:AddPhases.
// Phase 2 of code:Reallocate
element_t * ReplaceTable(element_t * newTable, count_t newTableSize);

// Utility function to delete old table (as returned by code:ReplaceTable).
// Phase 3 of code:Reallocate - it is split to support code:AddPhases.
// Phase 3 of code:Reallocate
void DeleteOldTable(element_t * oldTable);

// Utility function that does not call code:CheckGrowth.
Expand Down Expand Up @@ -502,92 +497,6 @@ class EMPTY_BASES_DECL SHash : public TRAITS
}
};

// Wrapper and holder for adding an element to the hash table. Useful for Add operations that have to happen
// under a rare lock that does not allow call out into host.
// There are 3 phases:
// 1. code:PreallocateForAdd ... Can allocate memory (calls into host).
// 2. code:Add ... Adds one element (does NOT call into host).
// or code:AddNothing_PublishPreallocatedTable ... Publishes the pre-allocated memory from step #1 (if any).
// 3. code:DeleteOldTable (or destructor) ... Can delete the old memory (calls into host).
// Example:
// CrstHolder lockAdd(&crstLockForAdd); // Serialize all Add operations.
// HostAssemblyMap::AddPhases addCall;
// addCall.PreallocateForAdd(&shash); // 1. Allocates memory for one Add call (if needed). addCall serves as holder for the allocated memory.
// {
// // We cannot call out into host from ForbidSuspend region (i.e. no allocations/deallocations).
// ForbidSuspendThreadHolder suspend; // Required by the 'special' read-lock
// {
// CrstHolder lock(&crstLock);
// if (some_condition)
// { // 2a. Add item. This may replace SHash inner table with the one pre-allocated in step 1.
// addCall.Add(shashItem);
// }
// else
// { // 2b. Skip adding item. This may replace SHash inner table with the one pre-allocated in step 1.
// addCall.AddNothing_PublishPreallocatedTable();
// }
// }
// }
// addCall.DeleteOldTable(); // 3. Cleanup old table memory from shash (if it was replaced by pre-allocated table in step 2).
// // Note: addCall destructor would take care of deleting the memory as well.
class AddPhases
{
public:
AddPhases();
~AddPhases();

// Prepares object for one call to code:Add. Pre-allocates new table memory if needed, does not publish
// the table yet (it is kept ready only in this holder for call to code:Add).
// Calls out into host.
void PreallocateForAdd(SHash * pHash);

// Add an element to the hash table. This will never replace an element; multiple elements may be stored
// with the same key.
// Will use/publish pre-allocated memory from code:PreallocateForAdd.
// Does not call out into host.
// Only one Add* method can be called once per object! (Create a new object for each call)
void Add(const element_t & element);

// Element will not be added to the hash table.
// Will use/publish pre-allocated memory from code:PreallocateForAdd.
// Does not call out into host.
// Only one Add* method can be called once per object! (Create a new object for each call)
void AddNothing_PublishPreallocatedTable();

// Deletes old table if it was replaced by call to code:Add or code:AddNothing_PublishPreallocatedTable.
// Calls out into host.
void DeleteOldTable();

private:
SHash * m_pHash;
element_t * m_newTable;
count_t m_newTableSize;
element_t * m_oldTable;

#ifdef _DEBUG
PTR_element_t dbg_m_table;
count_t dbg_m_tableSize;
count_t dbg_m_tableCount;
count_t dbg_m_tableOccupied;
count_t dbg_m_tableMax;
BOOL dbg_m_fAddCalled;
#endif //_DEBUG
}; // class SHash::AddPhases

// Adds an entry to the hash table according to the guidelines above for
// avoiding a callout to the host while the read lock is held.
// Returns true if elem was added to the map, otherwise false.
// When elem was not added (false is returned), and if ppStoredElem is non-null,
// then it is set to point to the value in the map.
template <typename LockHolderT,
typename AddLockHolderT,
typename LockT,
typename AddLockT>
bool CheckAddInPhases(
element_t const & elem,
LockT & lock,
AddLockT & addLock,
IUnknown * addRefObject = nullptr);

private:

Expand Down
Loading