From b8f26afeb9d5db98872d238568e09dc59428edf8 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Tue, 21 Jul 2020 13:02:23 +0200 Subject: [PATCH 1/4] add fast platform specializations for existsAnd!T Adds Exception-free versions of existsAnd!isFile and existsAnd!isDir for Windows and Posix platforms. On Windows this drastically increases performance of the resolveImportLocation function which is extensively called on startup, taking down execution time from previously around 60ms per call to <1ms Greatly improves performance of first pass on newly opened files, however resolveImportLocation could be optimized even further in future commits. --- src/dsymbol/modulecache.d | 86 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/src/dsymbol/modulecache.d b/src/dsymbol/modulecache.d index 8a298b4..a496b1b 100644 --- a/src/dsymbol/modulecache.d +++ b/src/dsymbol/modulecache.d @@ -436,10 +436,92 @@ private: /// Wrapper to check some attribute of a path, ignoring errors /// (such as on a broken symlink). -private static bool existsAnd(alias fun)(string fn) +private static bool existsAnd(alias fun)(string file) { try - return fun(fn); + return fun(file); catch (FileException e) return false; } + +/// same as getAttributes without throwing +/// Returns: true if exists, false otherwise +private static bool getFileAttributesFast(R)(R name, uint* attributes) +{ + version (Windows) + { + import std.internal.cstring : tempCStringW; + import core.sys.windows.winnt : INVALID_FILE_ATTRIBUTES, + FILE_ATTRIBUTE_DIRECTORY; + import core.sys.windows.winbase : GetFileAttributesW; + + auto namez = tempCStringW(name); + static auto trustedGetFileAttributesW(const(wchar)* namez) @trusted + { + return GetFileAttributesW(namez); + } + *attributes = trustedGetFileAttributesW(namez); + return *attributes != INVALID_FILE_ATTRIBUTES; + } + else version (Posix) + { + import core.sys.posix.sys.stat : stat, stat_t, S_IFMT, S_IFREG, S_IFDIR; + import std.internal.cstring : tempCString; + + auto namez = tempCString(name); + static auto trustedStat(const(char)* namez, out stat_t statbuf) @trusted + { + return stat(namez, &statbuf); + } + + stat_t statbuf; + const ret = trustedStat(namez, statbuf) == 0; + *attributes = statbuf.st_mode; + return ret; + } + else + { + static assert(false, "Unimplemented getAttributes check"); + } +} + +private static bool existsAnd(alias fun : isFile)(string file) +{ + uint attributes; + if (!getFileAttributesFast(file, &attributes)) + return false; + return attrIsFile(attributes); +} + +private static bool existsAnd(alias fun : isDir)(string file) +{ + uint attributes; + if (!getFileAttributesFast(file, &attributes)) + return false; + return attrIsDir(attributes); +} + +version (Windows) +{ + unittest + { + assert(existsAnd!isFile(`C:\Windows\regedit.exe`)); + assert(existsAnd!isDir(`C:\Windows`)); + assert(!existsAnd!isDir(`C:\Windows\regedit.exe`)); + assert(!existsAnd!isDir(`C:\SomewhereNonExistant\nonexistant.exe`)); + assert(!existsAnd!isFile(`C:\SomewhereNonExistant\nonexistant.exe`)); + assert(!existsAnd!isFile(`C:\Windows`)); + } +} +else version (Posix) +{ + unittest + { + assert(existsAnd!isFile(`/bin/true`)); + assert(existsAnd!isDir(`/bin`)); + assert(!existsAnd!isDir(`/bin/true`)); + assert(!existsAnd!isDir(`/nonexistant_dir/__nonexistant`)); + assert(!existsAnd!isFile(`/nonexistant_dir/__nonexistant`)); + assert(!existsAnd!isFile(`/bin`)); + } +} From c07d7519b79a13a4474fe8f5d9980224f089ff10 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Tue, 21 Jul 2020 13:05:00 +0200 Subject: [PATCH 2/4] return early for exactly matching D imports This now prioritizes earlier import paths rather than later import paths for .d filename matches. Before this was inconsistent with the other types (.di, package.di, package.d) where only the first import path would matter but where with .d files the last import path would matter --- src/dsymbol/modulecache.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dsymbol/modulecache.d b/src/dsymbol/modulecache.d index a496b1b..30fb60c 100644 --- a/src/dsymbol/modulecache.d +++ b/src/dsymbol/modulecache.d @@ -313,7 +313,7 @@ struct ModuleCache string dotD = dotDi[0 .. $ - 1]; string withoutSuffix = dotDi[0 .. $ - 3]; if (existsAnd!isFile(dotD)) - alternatives = dotD ~ alternatives; + return istring(dotD); // return early for exactly matching .d files else if (existsAnd!isFile(dotDi)) alternatives ~= dotDi; else if (existsAnd!isDir(withoutSuffix)) From 0009f5bb23778f30a898ea710267b1382eebc58f Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Tue, 21 Jul 2020 14:22:53 +0200 Subject: [PATCH 3/4] perform less IO in resolveImportLocation Less filesystem access, remove a lot of memory allocation, make code simpler to understand --- src/dsymbol/modulecache.d | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/dsymbol/modulecache.d b/src/dsymbol/modulecache.d index 30fb60c..244ba26 100644 --- a/src/dsymbol/modulecache.d +++ b/src/dsymbol/modulecache.d @@ -298,16 +298,20 @@ struct ModuleCache assert(moduleName !is null, "module name is null"); if (isRooted(moduleName)) return istring(moduleName); - string[] alternatives; + string alternative; foreach (importPath; importPaths[]) { auto path = importPath.path; - if (path.existsAnd!isFile) + // import path is a filename + // first check string if this is a feasable path (no filesystem usage) + if (path.stripExtension.endsWith(moduleName) + && path.existsAnd!isFile) { - if (path.stripExtension.endsWith(moduleName)) - alternatives ~= path; + // prefer exact import names above .di/package.d files + return istring(path); } - else + // no exact matches and no .di/package.d matches either + else if (!alternative.length) { string dotDi = buildPath(path, moduleName) ~ ".di"; string dotD = dotDi[0 .. $ - 1]; @@ -315,21 +319,26 @@ struct ModuleCache if (existsAnd!isFile(dotD)) return istring(dotD); // return early for exactly matching .d files else if (existsAnd!isFile(dotDi)) - alternatives ~= dotDi; + alternative = dotDi; else if (existsAnd!isDir(withoutSuffix)) { string packagePath = buildPath(withoutSuffix, "package.di"); - if (existsAnd!isFile(packagePath)) - { - alternatives ~= packagePath; - continue; - } if (existsAnd!isFile(packagePath[0 .. $ - 1])) - alternatives ~= packagePath[0 .. $ - 1]; + alternative = packagePath[0 .. $ - 1]; + else if (existsAnd!isFile(packagePath)) + alternative = packagePath; } } + // we have a potential .di/package.d file but continue searching for + // exact .d file matches to use instead + else + { + string dotD = buildPath(path, moduleName) ~ ".d"; + if (existsAnd!isFile(dotD)) + return istring(dotD); // return early for exactly matching .d files + } } - return alternatives.length > 0 ? istring(alternatives[0]) : istring(null); + return alternative.length > 0 ? istring(alternative) : istring(null); } auto getImportPaths() const From 70cc464facb35a6afe34acdbe67b1f71a5278695 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Wed, 22 Jul 2020 07:09:47 +0200 Subject: [PATCH 4/4] Remove unnecessary imports from getFileAttributes --- src/dsymbol/modulecache.d | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/dsymbol/modulecache.d b/src/dsymbol/modulecache.d index 244ba26..36914fa 100644 --- a/src/dsymbol/modulecache.d +++ b/src/dsymbol/modulecache.d @@ -460,8 +460,7 @@ private static bool getFileAttributesFast(R)(R name, uint* attributes) version (Windows) { import std.internal.cstring : tempCStringW; - import core.sys.windows.winnt : INVALID_FILE_ATTRIBUTES, - FILE_ATTRIBUTE_DIRECTORY; + import core.sys.windows.winnt : INVALID_FILE_ATTRIBUTES; import core.sys.windows.winbase : GetFileAttributesW; auto namez = tempCStringW(name); @@ -474,7 +473,7 @@ private static bool getFileAttributesFast(R)(R name, uint* attributes) } else version (Posix) { - import core.sys.posix.sys.stat : stat, stat_t, S_IFMT, S_IFREG, S_IFDIR; + import core.sys.posix.sys.stat : stat, stat_t; import std.internal.cstring : tempCString; auto namez = tempCString(name);