Page MenuHomePhabricator

Add 'lint.limit' option and use it in linters
AbandonedPublic

Authored by hach-que on Nov 19 2013, 7:17 AM.
Tags
None
Referenced Files
F13344197: D7606.diff
Fri, Jun 21, 7:01 AM
F13329197: D7606.diff
Sun, Jun 16, 10:12 PM
F13316842: D7606.diff
Thu, Jun 13, 8:41 AM
F13315779: D7606.id17163.diff
Wed, Jun 12, 6:29 PM
F13315724: D7606.id.diff
Wed, Jun 12, 5:42 PM
F13315686: D7606.diff
Wed, Jun 12, 5:16 PM
F13315545: D7606.diff
Wed, Jun 12, 3:14 PM
F13302409: D7606.diff
Sat, Jun 8, 5:03 AM

Details

Summary

This adds a 'lint.limit' option that can be set on an .arcconfig or user-basis. The reason for setting this at an .arcconfig level instead of an .arclint level is that the CPU is very likely to vary between different machines, and so this should be a setting that the user controls.

The setParallelisationLimit method is called with the configuration value, and defaults to 8. The C# linter and the Future linter use this value for their limits when executing futures.

Test Plan

Did arc set-config lint.limit <value> with various values and verified that it changes how many commands were executed at once.

Diff Detail

Branch
linter-limit
Lint
Lint Passed
Unit
Test Failures

Event Timeline

In the past, we had a method on ExecFuture called:

limitPerCore($factor = 1)

Maybe we should restore that? A simple implementation would be:

public function limitPerCore($factor = 1) {
  $cores_on_this_machine = 4;
  return $this->limit((int)ceil($factor * $cores_on_this_machine));
}

A more advanced implementation could try to figure out the number of cores instead of guessing that it's 4.

The logic here is that when you limit(), the "right" number is almost always one process per core, or some factor thereof.

I'll accept either a hardcoded 8 (or similar) or a limitPerCore() version of this, but I don't want to introduce "lint.limit" as a per-project value because the "right" level of parallelism seems most strongly a function of the machine.

I've found that Linux is a lot better at parallising this kind of work than Windows. On Linux with 4 cores I can quite comfortably set the limit to 50 and all the execs are finished within 15 seconds (most in 9 seconds). Windows on the other hand struggles with parallising workload. So I don't think setting it to either a fixed number or based on CPU cores will really work; the kernel's ability to parallise tasks has a huge impact on what number is appropriate.

Is there a way to put this config in arcrc? I don't intend on this setting being set per-project.

Doesn't that make putting this in project config worse? I'm completely fine with an implementation like:

if (phutil_is_windows()) {
  // lol windows
  return 2;
}
return $actual_core_count;

Turns out that I don't know what I'm talking about. Running with lint.limit 50 is identical to running with lint.limit 8 on this Linux machine.