Page MenuHomePhabricator

Load balancing for LTO
Needs RevisionPublic

Authored by david2050 on Apr 9 2019, 7:32 PM.

Details

Summary

Use bitcode size as an estimated for compilation time and sort
compilation tasks in order decreasing sizee to provide better
load balance when there are large number of taks.

Diff Detail

Event Timeline

david2050 created this revision.Apr 9 2019, 7:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2019, 7:32 PM
fhahn added a subscriber: fhahn.Apr 10 2019, 5:04 AM
mehdi_amini added inline comments.Apr 11 2019, 5:48 PM
lib/LTO/LTO.cpp
1301

The ThinLink has access to much fine grain data: like number of function and number of instructions. Have you looked into using this?

1312

FYI: https://github.com/llvm/llvm-project/blob/master/llvm/lib/LTO/ThinLTOCodeGenerator.cpp#L923

I used a separate index vector in ThinLTOCodeGenerator to do the same, it may make the code a bit easier to track (less pair<> and .first->second kind of thing).

david2050 planned changes to this revision.Apr 11 2019, 6:54 PM
david2050 marked 2 inline comments as done.
david2050 added inline comments.
lib/LTO/LTO.cpp
1301

I did not explore alternatives. This was effective for the workloads that motivated it so I thought I would share.

1312

Your implementation is cleaner. I may not have time to retest.

david2050 updated this revision to Diff 194799.Apr 11 2019, 7:41 PM

copy & paste & modify :-) from Mehdi pointerwd

ormris added a subscriber: ormris.Apr 12 2019, 9:36 AM

Hello! Would anyone be willing to review this patch? Is there anything that could be tweaked, maybe putting this behavior behind an option, that would make the change more amenable to any reviewers?

Thanks! I think this makes sense, and since this is now the same as what is done in the old LTO API (ThinLTOCodeGenerator.cpp), thinking it would be better to extract into a common helper in LTO.cpp that is called from both places (that way if we find a way to improve the sorting, e.g. some of the other metrics @mehdi_amini mentioned, it only has to be changed in one place).

mehdi_amini accepted this revision.Jun 28 2019, 4:03 PM

LGTM, but it'd be nice if @tejohnson or @pcc would also approve.

This revision is now accepted and ready to land.Jun 28 2019, 4:03 PM

LGTM, but it'd be nice if @tejohnson or @pcc would also approve.

I added a suggestion a couple hours ago, just for commoning the code between the two LTO APIs.

mehdi_amini requested changes to this revision.Jun 28 2019, 4:15 PM

LGTM, but it'd be nice if @tejohnson or @pcc would also approve.

I added a suggestion a couple hours ago, just for commoning the code between the two LTO APIs.

Looks like I was so slow to type that our answers crossed ;)

It makes sense to me! Thanks.

This revision now requires changes to proceed.Jun 28 2019, 4:15 PM