Ben Gamari pushed to branch wip/T26009 at Glasgow Haskell Compiler / GHC Commits: 6fbb05cf by Ben Gamari at 2025-05-19T17:10:35-04:00 rts/linker/PEi386: Fix incorrect use of break in nested for Previously the happy path of PEi386 used `break` in a double-`for` loop resulting in redundant calls to `LoadLibraryEx`. Fixes #26052. - - - - - d7c918eb by Ben Gamari at 2025-05-19T17:11:00-04:00 rts: Mark const arguments - - - - - bbc9c637 by Ben Gamari at 2025-05-19T17:11:26-04:00 rts/linker/PEi386: Don't repeatedly load DLLs Previously every DLL-imported symbol would result in a call to `LoadLibraryEx`. This ended up constituting over 40% of the runtime of `ghc --interactive -e 42` on Windows. Avoid this by maintaining a hash-set of loaded DLL names, skipping the call if we have already loaded the requested DLL. Addresses #26009. - - - - - 4 changed files: - rts/PathUtils.c - rts/PathUtils.h - rts/linker/PEi386.c - rts/linker/PEi386.h Changes: ===================================== rts/PathUtils.c ===================================== @@ -13,7 +13,7 @@ #include <wchar.h> #endif -pathchar* pathdup(pathchar *path) +pathchar* pathdup(const pathchar *path) { pathchar *ret; #if defined(mingw32_HOST_OS) @@ -26,7 +26,7 @@ pathchar* pathdup(pathchar *path) return ret; } -pathchar* pathdir(pathchar *path) +pathchar* pathdir(const pathchar *path) { pathchar *ret; #if defined(mingw32_HOST_OS) @@ -50,7 +50,7 @@ pathchar* pathdir(pathchar *path) return ret; } -pathchar* mkPath(char* path) +pathchar* mkPath(const char* path) { #if defined(mingw32_HOST_OS) size_t required = mbstowcs(NULL, path, 0); @@ -66,7 +66,7 @@ pathchar* mkPath(char* path) #endif } -HsBool endsWithPath(pathchar* base, pathchar* str) { +HsBool endsWithPath(const pathchar* base, const pathchar* str) { int blen = pathlen(base); int slen = pathlen(str); return (blen >= slen) && (0 == pathcmp(base + blen - slen, str)); ===================================== rts/PathUtils.h ===================================== @@ -37,9 +37,9 @@ #include "BeginPrivate.h" -pathchar* pathdup(pathchar *path); -pathchar* pathdir(pathchar *path); -pathchar* mkPath(char* path); -HsBool endsWithPath(pathchar* base, pathchar* str); +pathchar* pathdup(const pathchar *path); +pathchar* pathdir(const pathchar *path); +pathchar* mkPath(const char* path); +HsBool endsWithPath(const pathchar* base, const pathchar* str); #include "EndPrivate.h" ===================================== rts/linker/PEi386.c ===================================== @@ -378,7 +378,7 @@ static size_t makeSymbolExtra_PEi386( #endif static void addDLLHandle( - pathchar* dll_name, + const pathchar* dll_name, HINSTANCE instance); static bool verifyCOFFHeader( @@ -427,8 +427,52 @@ const int default_alignment = 8; the pointer as a redirect. Essentially it's a DATA DLL reference. */ const void* __rts_iob_func = (void*)&__acrt_iob_func; +/* + * Note [Avoiding repeated DLL loading] + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * As LoadLibraryEx tends to be expensive and addDLL_PEi386 is called on every + * DLL-imported symbol, we use a hash-map to keep track of which DLLs have + * already been loaded. This hash-map is keyed on the dll_name passed to + * addDLL_PEi386 and is mapped to its HINSTANCE. This serves as a quick check + * to avoid repeated calls to LoadLibraryEx for the identical DLL. See #26009. + */ + +typedef struct { + HashTable *hash; +} LoadedDllCache; + +LoadedDllCache loaded_dll_cache; + +static void initLoadedDllCache(LoadedDllCache *cache) { + cache->hash = allocHashTable(); +} + +static int hash_path(const HashTable *table, StgWord w) +{ + const pathchar *key = (pathchar*) w; + return hashBuffer(table, key, sizeof(pathchar) * wcslen(key)); +} + +static int compare_path(StgWord key1, StgWord key2) +{ + return wcscmp((pathchar*) key1, (pathchar*) key2) == 0; +} + +static void addLoadedDll(LoadedDllCache *cache, const pathchar *dll_name, HINSTANCE instance) +{ + insertHashTable_(cache->hash, (StgWord) dll_name, instance, hash_path); +} + +static HINSTANCE isDllLoaded(const LoadedDllCache *cache, const pathchar *dll_name) +{ + void *result = lookupHashTable_(cache->hash, (StgWord) dll_name, hash_path, compare_path); + return (HINSTANCE) result; +} + void initLinker_PEi386(void) { + initLoadedDllCache(&loaded_dll_cache); + if (!ghciInsertSymbolTable(WSTR("(GHCi/Ld special symbols)"), symhash, "__image_base__", GetModuleHandleW (NULL), HS_BOOL_TRUE, @@ -455,7 +499,7 @@ void exitLinker_PEi386(void) static OpenedDLL* opened_dlls = NULL; /* Adds a DLL instance to the list of DLLs in which to search for symbols. */ -static void addDLLHandle(pathchar* dll_name, HINSTANCE instance) { +static void addDLLHandle(const pathchar* dll_name, HINSTANCE instance) { IF_DEBUG(linker, debugBelch("addDLLHandle(%" PATH_FMT ")...\n", dll_name)); /* At this point, we actually know what was loaded. @@ -797,11 +841,20 @@ uint8_t* getSymShortName ( COFF_HEADER_INFO *info, COFF_symbol* sym ) } const char * -addDLL_PEi386( pathchar *dll_name, HINSTANCE *loaded ) +addDLL_PEi386( const pathchar *dll_name, HINSTANCE *loaded ) { /* ------------------- Win32 DLL loader ------------------- */ IF_DEBUG(linker, debugBelch("addDLL; dll_name = `%" PATH_FMT "'\n", dll_name)); + // See Note [Avoiding repeated DLL loading] + HINSTANCE instance = isDllLoaded(&loaded_dll_cache, dll_name); + if (instance) { + if (loaded) { + *loaded = instance; + } + return NULL; + } + /* The file name has no suffix (yet) so that we can try both foo.dll and foo.drv @@ -820,7 +873,6 @@ addDLL_PEi386( pathchar *dll_name, HINSTANCE *loaded ) const DWORD flags[] = { LOAD_LIBRARY_SEARCH_USER_DIRS | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS, 0 }; /* Iterate through the possible flags and formats. */ - HINSTANCE instance; for (int cFlag = 0; cFlag < 2; cFlag++) { for (int cFormat = 0; cFormat < 4; cFormat++) { snwprintf(buf, bufsize, formats[cFormat], dll_name); @@ -830,16 +882,16 @@ addDLL_PEi386( pathchar *dll_name, HINSTANCE *loaded ) goto error; } } else { - break; /* We're done. DLL has been loaded. */ + goto loaded; /* We're done. DLL has been loaded. */ } } } - /* Check if we managed to load the DLL. */ - if (instance == NULL) { - goto error; - } + // We failed to load + goto error; +loaded: + addLoadedDll(&loaded_dll_cache, dll_name, instance); addDLLHandle(buf, instance); if (loaded) { *loaded = instance; ===================================== rts/linker/PEi386.h ===================================== @@ -45,7 +45,7 @@ typedef struct _COFF_HEADER_INFO { void initLinker_PEi386( void ); void exitLinker_PEi386( void ); -const char * addDLL_PEi386( pathchar *dll_name, HINSTANCE *instance ); +const char * addDLL_PEi386( const pathchar *dll_name, HINSTANCE *instance ); void freePreloadObjectFile_PEi386( ObjectCode *oc ); bool checkAndLoadImportLibrary( pathchar* arch_name, char* member_name, FILE* f); View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/6a1cf6ffb6d6d6e60a5a6034fb0ad4e... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/6a1cf6ffb6d6d6e60a5a6034fb0ad4e... You're receiving this email because of your account on gitlab.haskell.org.