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
#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.
