[PATCH] D47565: Fix /WholeArchive bug.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lld/COFF/Driver.cpp:1248-1249
+      return true;
+    for (auto *Arg : Args.filtered(OPT_wholearchive_file))
+      if (sys::fs::equivalent(Path, Arg->getValue()))
+        return true;
----------------
smeenai wrote:
> ruiu wrote:
> > rnk wrote:
> > > sys::fs::equivalent internally opens and closes a file handle for the purpose of doing a stat and comparing inodes. This will end up doing `O(inputs * wholearchiveinputs)` equivalency tests, and linkers have many input object files. Is there a way to avoid that?
> > That's right, but I don't think there's a way to avoid that unless we implement some OS-dependent logic on our side. How operating system normalize pathname components depends on the operating system and the file system, and I believe the only reliable way to do that is to ask about it to the system.
> > 
> > Maybe we could cache stat's results along with filenames to reduce the number of system calls, but I'm not sure if we need it. If it turns out to be too slow, we could optimize, but probably we should do that later when it becomes a real problem.
> > 
> > In reality, what is the largest number of files you can think of you want to pass to the linker? For hundreds of files, this is probably fine. If you pass thousands of files, this is probably slow.
> We have a library that's built from ~5,000 object files, so we care about the performance here :) I can try to get some numbers later today.
Right, O(n) stats should be fine, just use llvm::sys::fs::getUniqueId or whatever it's called, and it will be fine. That gets the inode (or its local equivalent).


https://reviews.llvm.org/D47565