Skip to content

Commit 1a50d04

Browse files
committed
[lld] check cache before real_path in loadDylib
In llvm#137649 symlink resolution was added when loading dylibs. This introduced a performance regression when linking with a large number of inputs with LC_LOAD_DYLIB commands due to the syscall overhead of realpath. Refactor the change to be closer to the original: - first check if the given path is in the cache - if not, resolve it and check again - update cache entries of both paths to point to the same dylib This mitigates the regression as we do not incur the realpath cost for every loadDylib call, only once per unique path.
1 parent 8708c42 commit 1a50d04

File tree

1 file changed

+20
-6
lines changed

1 file changed

+20
-6
lines changed

lld/MachO/DriverUtils.cpp

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,19 +229,31 @@ static DenseMap<CachedHashStringRef, DylibFile *> loadedDylibs;
229229

230230
DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
231231
bool isBundleLoader, bool explicitlyLinked) {
232-
// Frameworks can be found from different symlink paths, so resolve
233-
// symlinks before looking up in the dylib cache.
234-
SmallString<128> realPath;
235-
std::error_code err = fs::real_path(mbref.getBufferIdentifier(), realPath);
236-
CachedHashStringRef path(!err ? uniqueSaver().save(StringRef(realPath))
237-
: mbref.getBufferIdentifier());
232+
CachedHashStringRef path(mbref.getBufferIdentifier());
238233
DylibFile *&file = loadedDylibs[path];
239234
if (file) {
240235
if (explicitlyLinked)
241236
file->setExplicitlyLinked();
242237
return file;
243238
}
244239

240+
// Frameworks can be found from different symlink paths, so resolve
241+
// symlinks and look up in the dylib cache.
242+
DylibFile *&realfile = file;
243+
SmallString<128> realPath;
244+
std::error_code err = fs::real_path(mbref.getBufferIdentifier(), realPath);
245+
if (!err) {
246+
CachedHashStringRef resolvedPath(uniqueSaver().save(StringRef(realPath)));
247+
realfile = loadedDylibs[resolvedPath];
248+
if (realfile) {
249+
if (explicitlyLinked)
250+
realfile->setExplicitlyLinked();
251+
252+
file = realfile;
253+
return realfile;
254+
}
255+
}
256+
245257
DylibFile *newFile;
246258
file_magic magic = identify_magic(mbref.getBuffer());
247259
if (magic == file_magic::tapi_file) {
@@ -253,6 +265,7 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
253265
}
254266
file =
255267
make<DylibFile>(**result, umbrella, isBundleLoader, explicitlyLinked);
268+
realfile = file;
256269

257270
// parseReexports() can recursively call loadDylib(). That's fine since
258271
// we wrote the DylibFile we just loaded to the loadDylib cache via the
@@ -268,6 +281,7 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
268281
magic == file_magic::macho_executable ||
269282
magic == file_magic::macho_bundle);
270283
file = make<DylibFile>(mbref, umbrella, isBundleLoader, explicitlyLinked);
284+
realfile = file;
271285

272286
// parseLoadCommands() can also recursively call loadDylib(). See comment
273287
// in previous block for why this means we must copy `file` here.

0 commit comments

Comments
 (0)