[PATCH] D115416: [lld-macho] Prevent writing map files on the critical path

[Vincent Lee via Phabricator via llvm-commits]

thevinster added a comment.

> I'm assuming writeOutputFile takes longer than writeMapAsync anyway

Profiling on my end shows that `writeMapFile` actually takes longer than `writeOutputFile` and on some cases longer than `finalizeLinkEditSegment`. I personally believe we get more wins from if we did `writeMapFile` before `finalizeLinkEditSegment`. But, if we plan on having `writeMapFile` depend on contents in `finalizeLinkEditSegment` in the future, I'm happy to move it after `finalizeLinkEditSegment`.

> dont you want std::future<void> writeMapAsync (std::async(std::launch::async, []{ writeMapFile(); }));  ?

By not providing the policy, it lets the system decide whether it should be async or deferred. The async policy is preferred, but the system will choose deferred if the system is running out of memory and can't spawn another thread in efforts to not hard crash. I think we can agree that it's better not to crash if it means having to not run it in parallel? Also, the lambda isn't necessary at all. It's working without the lambda on my end, reference docs don't specify that it must be wrapped in a lambda.

> so I don't think the get() call below is necessary?

In the majority of cases, it's probably not necessary, but I think it's better to be safer than sorry. If the implementation changes behind the scenes for us and starts producing undefined behaviors, then it might not be obvious where crashes originate from right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115416