[apparmor] [PATCH] parser: add basic support for parallel compiles and loads

[John Johansen]

On 11/29/2015 03:11 PM, Christian Boltz wrote:
> Hello,
> 
> Am Samstag, 28. November 2015 schrieb John Johansen:
>> On 11/28/2015 01:54 PM, Christian Boltz wrote:
>>> Am Samstag, 28. November 2015 schrieb John Johansen:
> ...
>>> So the parser will error out if a too big job number is given _and_
>>> if there are enough profiles to load (otherwise the number gets
>>> reduced to the number of available profiles/jobs).
>>
>> No only if the max number of jobs is being forced and is much larger
>> than the available cpus
> 
> Ah, I overlooked that your patch adds -j/--jobs _and_ --max-jobs.
> 
> BTW: Is it really useful to have two options? I didn't re-read the whole 
> patch, but both options sound like they do (nearly?) the same: Specify 
> how many threads should be used.
> 
hrmm well max-jobs was supposed to let you tune the maximum number to
keep someone from breaking the system

>>> In other words: I can use -j 10000 for quite a while, but it will
>>> break after I add another profile to my system.
>>
>> No.
>>
>> Max jobs only controls the number of parallel processes at any given
>> moment, the number of profiles have nothing to do with it. 
> 
> I'm especially looking at
> 
>>>> +	jobs_max = min(jobs_count, jobs_max);
> 
> and finally noticed that jobs_count also comes from a commandline 
> parameter (--jobs) - yesterday, I thought that jobs_count is the number 
> of jobs to do, as in "the number of profiles given in the commandline" 
> :-/  (Sorry!)
> 
> Ok, so it really doesn't depend on the number of profiles.
> 
> That said - the quoted line seems to be the only line in your patch that 
> actually uses jobs_count. (See the BTW above.)
> 

right, so the the idea being to take the least of jobs and max_jobs, and
once that decision is done run with just one value

>> If the number of jobs is too high, the processes are competing with
>> each other and builds will actually slow down. If the number gets
>> high enough, the parser can DOS the system consuming all resources.
> 
> Right, I "tried" that with apache processes on one of my servers and 
> learned that a lower limit results in better performance by having some 
> RAM left for cache ;-)
> 
>>> I'd prefer to reduce jobs_max to 8*n and print a warning.
>>>
>>> In C-like Pseudocode:
>>>
>>> +	if (jobs_max > 8*n) {
>>> +		WARN("%s: Invalid maximum number of jobs '%ld' > 8 * # of 
> cpus,
>>> reducing to %ld", +		       progname, jobs_max, n);
>>> +		jobs_max = 8*n;
>>> +	}
>>
>> meh, I guess we could do that, but again max_jobs 8 * # of cpus is
>> going to slow things down.
> 
> Feel free to use 4*n or whatever you think is a sane value (however if 
> the limit is 8*n, reducing to something else would be strange) - but 
> please don't error out on a too high value of --jobs or --max-jobs.
> (Erroring out would mean that a script written on a new computer could 
> fail on an old one with less processor cores.)
> 
> 
> Hmm, while re-reading the patch, I noticed that you use
> 
> +       {"max_jobs",            1, 0, 136},     /* no short option */
> ...
> +              "--max_jobs [n]          Set the maximum number of 
> threads\n"
> ...
> +       case 136:
> +               jobs_max = process_jobs_arg("max_jobs", optarg);
> 
> (notice the "_") while all other options are named --some-thing.
> If you really keep both options, please change that to --max-jobs.
> 
> 
So max-jobs does kind of feel like it could be replaced or at least
reworked atm.  As the 8 x cpus kind of takes its place.

We could just drop it or convert it to mean the number of times cpus.
Ie.  default to 8x, but allow a user to configure to 4x or 16x or
what ever