[PATCH] D25585: Add interface for querying physical hardware concurrency

[Teresa Johnson via llvm-commits]

On Fri, Oct 14, 2016 at 9:35 AM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2016-Oct-14, at 06:24, Teresa Johnson <tejohnson at google.com> wrote:
> >
> >
> >
> > On Thu, Oct 13, 2016 at 6:22 PM, Mehdi Amini <mehdi.amini at apple.com>
> wrote:
> >
> >> On Oct 13, 2016, at 5:25 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> >>
> >>>
> >>> On Oct 13, 2016, at 5:22 PM, Teresa Johnson <tejohnson at google.com>
> wrote:
> >>>
> >>>
> >>>
> >>> On Thu, Oct 13, 2016 at 5:14 PM, Mehdi Amini <mehdi.amini at apple.com>
> wrote:
> >>>
> >>> > On Oct 13, 2016, at 5:07 PM, Teresa Johnson <tejohnson at google.com>
> wrote:
> >>> >
> >>> > tejohnson added inline comments.
> >>> >
> >>> >
> >>> > ================
> >>> > Comment at: include/llvm/Support/Threading.h:121
> >>> > +  /// host system, otherwise falls back to
> thread::hardware_concurrency().
> >>> > +  unsigned hardware_physical_concurrency();
> >>> > }
> >>> > ----------------
> >>> > mehdi_amini wrote:
> >>> >> I think we may want to name it `hardware_coarse_concurrency`.
> Because:
> >>> >>
> >>> >> - This looks like expressing better what we're looking after.
> >>> >> - Hyperthreading is in some sense "physical concurrency", but
> sharing some resources.
> >>> >> - Other platforms may have something in between.
> >>> >>
> >>> > I thought about that name after you mentioned it on the prior review
> thread. But I felt that saying "physical concurrency" is a better
> expression for what it is actually trying to give you.  I think of the
> hyperthreading concurrency as "logical concurrency", vs physical
> concurrency due to physical cores.
> >>>
> >>> Hyperthreading is sharing some physical resources on the core, but
> have some other physical resources duplicated/dedicated, which is why I
> feel it is murky. But “good enough” as well...
> >>> Also, I don’t know enough the PowerPC or Sparc equivalent of
> hyper-threading to know how much they share/duplicate and what we would
> pick on these for instance.
> >>>
> >>> Right it is a bit murky. One reason I didn't like "coarse" is that
> coarse-grained parallelism relates to how closely the tasks
> communicate/synchronize
> >>
> >> Fair.
> >> In my mind “coarse” relates to the “size” of the task. But a few light
> and long-lived task could fit a “coarse” level of granularity.
> >>
> >> I don’t have anything better to qualify these tasks (“heavy” is kind of
> what I’m looking for, but I can’t translate this into an API name…).
> >
> > Asked Duncan (he’s good at naming usually), and he suggested:
> >
> > lightweight_hardware_concurrency(); // maximum number of resource
> available for threading
> > heavyweight_hardware_concurrency();  // number of dedicated hardware
> resource available for threading
> > hardware_concurrency(); // default to one of the other above, or in
> between, depending on the platform.
> >
> > I like heavyweight_hardware_concurrency. But I think it is better to
> keep hardware_concurrency to its current meaning, since that is in
> std::thread.
>
> Agreed.  It should match std::thread::hardware_concurrency when available.
>
> > Also, it seems like you would want to pick a type of concurrency (heavy
> vs light) based on knowledge about the tasks being parallelized. Eg. for
> ThinLTO backends we know they are memory intensive, so use
> heavyweight_hardware_concurrency. Depending on the platform, this could
> map to the number of physical cores (as I did here), or for other platforms
> it may be better to simply use the default hardware_concurrency.
> >
> > It seems like lightweight_hardware_concurrency is pretty much always
> the same as hardware_concurrency (the std::thread implementation).
>
> Not guaranteed to be the same though.  std::thread::hardware_concurrency()
> is an implementation-dependent hint, and doesn't necessarily even match
> either of the two others.
>
> (I don't have a strong opinion on what you should do, besides having
> hardware_concurrency match std::thread.)
>
> > What if for now I renamed this to heavyweight_hardware_concurrency, and
> then the follow on ThinLTO patch will use that for BE parallelism. We could
> consider adding in additional flavors if the need ever arises.
>

Committed rename of hardware_physical_concurrency to
heavyweight_hardware_concurrency in r284390. Will leave
lightweight_hardware_concurrency to when there is a need for it, since I
don't have a clear idea of what it should do other than map it directly to
std::thread::hardware_concurrency.

Teresa

>
> > Teresa
> >
> >
> >
> > —
> > Mehdi
> >
> >
> >
> > --
> > Teresa Johnson |       Software Engineer |     tejohnson at google.com |
> 408-460-2413
>
>


-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161017/f6a61a75/attachment.html>