Page MenuHomeFreeBSD

llvm/lld: damage control threading
ClosedPublic

Authored by mjg on Apr 2 2023, 12:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 18, 10:08 AM
Unknown Object (File)
Oct 6 2024, 5:33 AM
Unknown Object (File)
Oct 5 2024, 4:43 PM
Unknown Object (File)
Oct 3 2024, 4:58 AM
Unknown Object (File)
Oct 2 2024, 10:10 PM
Unknown Object (File)
Oct 2 2024, 9:59 AM
Unknown Object (File)
Oct 2 2024, 6:10 AM
Unknown Object (File)
Oct 2 2024, 5:23 AM
Subscribers

Details

Summary

As the comment says:

+ Damage control threading.
+

+ There are no heuristics to figure out how many threads makes sense to spawn,
+
all while rolling with all available cores hw threads starts being detrimental
+ to performance really early.
+

+ Work around by putting a hard cap unless the user explicitly requested a certain amount.
+

+ See https://discourse.llvm.org/t/avoidable-overhead-from-threading-by-default/69160
+
for more details.

With the link above you can see I brought it upstream, but it does not seem to be considered a problem despite their own results showing the machinery does not scale(!).

The recommendation is to make sure to pass --threads=1 which given the above state should probably be default instead, requiring people to provide a different argument if they *want* threading. An alternative "If some users don’t like the default, they can create a ld.lld shell script." does not make sense either -- add fork + exec overhead only to damage control lld's feature which was supposed to improve performance.

With admission that some degree of threading ultimately does help reduce total real time, the proposed simple patch is an easy middle ground: you still get the threading which provides *some* benefit in real time reduction, but is no longer happy to eat everything past laptop scale.

Note I proposed the idea upstream but it was not considered viable.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mjg requested review of this revision.Apr 2 2023, 12:48 PM
mjg updated this revision to Diff 119774.
mjg edited the summary of this revision. (Show Details)

Let's do this for now, then we can talk to upstream again about putting in some sensible value.

contrib/llvm-project/llvm/lib/Support/Threading.cpp
76

MaxThreadCount is an int, so this cast to unsigned can go away. The one below is only because for some reason upstream has decided that ThreadsRequested is unsigned.

This revision is now accepted and ready to land.Apr 3 2023, 3:42 PM
This revision was automatically updated to reflect the committed changes.