[PATCH] D147960: [lld-macho] Cache discovered dylib paths

[Keith Smiley via Phabricator via llvm-commits]

keith added inline comments.


================
Comment at: lld/MachO/DriverUtils.cpp:191-194
+  CachedHashStringRef key(dylibPath);
+  auto entry = resolvedDylibs.find(key);
+  if (entry != resolvedDylibs.end())
+    return entry->second;
----------------
int3 wrote:
> how do you feel about using a similar approach to `loadDylib` below where we take a reference to the map value and then modify it? Aside from saving us two lines of `resolvedDylibs[key] = file`, it also serves as a negative cache -- if the key exists but the value is an empty string, then we can just return `std::nullopt`
> 
> (I notice we don't actually do this negative cache checking in `loadDylib`, but I guess in that case it matters less since we don't expect that cache lookup to fail in the common error-free case)
I think I'm missing the C++-ism for how to do this, in this case the value isn't a pointer to a StringRef, can we still use it in that way or should it be changed to be that? Also right now a missing value / empty string won't ever get cached, do you think we should cache that in the fallthrough case to handle the negative cache case you're talking about?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147960/new/

https://reviews.llvm.org/D147960