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

[Christian Boltz]

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.

> > 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.)

> 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.


Regards,

Christian Boltz
-- 
> Wenn Du KDE 1 an ein vier- und KDE 2 an ein dreibeiniges Schwein
> bindest und beide gleichzeitig trittst, läuft KDE 1 schneller. F'up.
Wenn du beide Schweine gleichzeitig trittst, landest du auf dem Hintern,
und dann bleiben beide Schweine vor Lachen stehn ;-)
[Robin S. Socha und André Schneider in dcoulm]