[PATCH] D27768: COFF: Open and map input files asynchronously on Windows.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D27768#622639, @ruiu wrote:

> Which is actually slow, open or mmap?


The MSVC profiler took 7510 "samples" inside getOpenFileImpl (which just maps the file) as compared to 7683 inside getFileAux (which opens the file then calls getOpenFileImpl).

> My wild guess is that mmap is slow. If so, can we open a file to get an error early and then mmap? It will simplify std::future<MBErrPair> to std::future<MemoryBuffer>.

That's a good point, I'll do that. Although we could fail to map the file, it's probably fine to exit on the thread since that's less likely than open failing.



================
Comment at: lld/COFF/Driver.cpp:794
   Symtab.addCombinedLTOObjects();
+  run();
 
----------------
ruiu wrote:
> Is this enough to run this only once? In the above code we repeat until converge.
We would only need to run `addCombinedLTOObjects` again if the native COFF file contained a compiler-generated reference to a bitcode symbol. This is unlikely to occur in the COFF linker because the compiler will generally only create references to the runtime library, which of course MSVC ships as regular object files. We previously tried to handle that situation by reporting an error (see SymbolTable.cpp:380 in the old code), but that code was untested. I didn't think it was worth it to preserve the error as this is basically a "don't care" situation, but I guess it wouldn't be too hard to do that and write a test.


================
Comment at: lld/COFF/InputFiles.h:86
 
   // Returns an input file for a given symbol. A null pointer is returned if we
   // have already returned the same input file. (So that we don't instantiate
----------------
ruiu wrote:
> Update comment.
Will do.


https://reviews.llvm.org/D27768