[node-core-library][rush] Support weighted async concurrency

[dmichon-msft]

Summary

Updates Async.forEachAsync with the ability to have variable concurrency based on task weights.

Updates Rush to use the feature for phased build concurrency.

The motivation for this feature is that when distributing tasks across available CPUs, sometimes a task may only use the target CPU some of the time (e.g. a network request) and therefore contributes a value <1 to the overall concurrency limit, and conversely sometimes a task may spin up multiple CPU-intensive threads and therefore contributes a value >1 to the overall concurrency limit.

Details

Adds a new option useWeight: true to Async.forEachAsync that is only available when the item type has a property weight?: number. If set, this value gets used for comparing available concurrency, instead of the default 1.

How it was tested

Added unit test for the scenario.

Impacted documentation

Just the docs for the method.

[octogonz]

This type algebra seems overcomplicated. I think is trying to say that IF the options specifies useWeight, THEN the entry must define a weight. I don't think it actually accomplishes that, but it is a slightly odd requirement given that both these properties are optional. If weight defaults to 1, then is it really important to have been able to specify it, even though we chose not to?

I think it might be better to simply say that "If useWeight=true, then forEachSync will use the weight property if present, otherwise the weight defaults to 1." The only realistic concern here is if old code randomly had a weight property, that suddenly gains new meaning in this release, but you've already solved that by defaulting useWeight to false.

This would also eliminate the need for a separate IAsyncParallelismOptionsWithWeight interface.

[dmichon-msft]

The check on the optional property is to ensure that the TEntry type does not have a property weight that is incompatible with type number. I originally considered writing this as an overload, but I'm generally leery of overloads in case something uses typeof Async.forEachAsync somewhere. Might be cleanest to just add a completely separate entry point.

[octogonz]

This needs more explanation. I had to read the code carefully to figure out what weight actually represents. My guess was that somehow affected the priority of the task (like Unix nice), but that was wrong. Also, the PR description and code comments never mention why someone would want this feature, i.e. what problem it solves.

[octogonz]

Try something like this:

  /**
   * Optionally used with the  {@link Async.mapAsync} and {@link Async.forEachAsync}
   * to limit amount of concurrency.
   * @remarks
   * By default, `concurrency` simply specifies the maximum number of promises that can be active
   * simultaneously.  However if `useWeight` is true, then behavior 
   * is different. See the {@link IAsyncParallelismOptions.useWeight} documentation for details.
   */
  concurrency?: number;

  /**
   * Set `useWeight` to true to configure the {@link IAsyncParallelismOptions.concurrency} 
   * setting to limit the total resource cost instead of limiting the total number of promises.
   * @remarks
   * By default, the `concurrency` specifies the maximum number of promises that can be
   * active simultaneously.  If promises have different resource costs, and those costs are known
   * in advance, then `useWeight` changes `concurrency` so that it limits the sum of the weights
   * of the entries passed to {@link Async.mapAsync} or {@link Async.forEachAsync}.
   * 
   * The {@link IAsyncEntry.weight} property is optional and defaults to 1.  (If all entries
   * use the default of 1, then the behavior is identical to `useWeight=false`.)
   * Weights can be fractional but not negative.
   */
  useWeight?: boolean;

  /**
   * If {@link Async.mapAsync} or {@link Async.forEachAsync} are used to parallelize tasks
   * that have different resource costs, and this property can be used to specify the resource
   * cost so that it can be limited by {@link IAsyncParallelismOptions.concurrency}.
   * @remarks
   * This property is ignored unless `useWeight` is true.  See the {@link 
   * IAsyncParallelismOptions.useWeight} documentation for details.
   */
  weight?: number

[octogonz]

Is weight the best word? Maybe concurrencyResourceCost (or at least concurrencyWeight)? It would be great of the naming gave some clue that concurrency and weight are part of the same feature.

Also we are asking to assign weight directly to somebody else's class/interface, so the more unique this name is, the less likely it is to accidentally conflict with an unrelated member.

[dmichon-msft]

The name weight was chosen purely to match its existing presence on Rush's Operation class, but that could be changed to whatever name seems to make the most sense.

I'm also open to having an altogether separate entry point for tasks whose CPU resource needs are not uniformly 1.

[octogonz]

{ weight?: number } must be declared as an interface, so that we have some place to document the weight property.

[iclanton]

Fix this?

[dmichon-msft]

I'm going to rework this PR with a different mechanism for addressing the same end goal.