Re: Adjusting jobserver size (was: Re: No follow up on patches to support newer glibc ?)

[Henrik Carlqvist]

I have added an updated patch to bug #51200 and hope that you will reconsider
adding the functionality into next release. It is true that SIGUSR is already
used for debug toggling, but the behavior of SIGUSR1 isn't changed to
decreasing number of jobs until a SIGUSR2 signal is received. So make will be
fully compatible with its current behavior unless it receives a SIGUSR2 when
the new features kicks in and also replaces debug toggling.

The changes in my latest patch are:

* Adopted to new directory structure

* Making sure that no signal unsafe functions are called from the signal
  functions. To really make sure of this no other functions are called from
  signal functions. This means that both increasing and decreasing the number
  of jobs will not be done until a job finishes.

Best regards Henrik

On Sat, 07 Apr 2018 16:46:00 -0400
Paul Smith  wrote:

> On Thu, 2018-04-05 at 00:26 +0200, Henrik Carlqvist wrote:
> > On Wed, 04 Apr 2018 15:42:51 -0400
> > Paul Smith  wrote:
> > > It does look like we need to make a new release soon.
> > 
> > If so, is there anything I can do to get the functionality of my
> > contributed patch in bug #51200 into the upcoming new release?
> 
> Thanks for the reminder Henrik.  For those interested, the link is:
> https://savannah.gnu.org/bugs/index.php?51200
> 
> I'll confess I'm on the fence about this.  On the one hand I could
> imagine where it would be useful.
> 
> On the other hand, it's a complex change (I'm not convinced that your
> implementation is complete: for example, it's not immediately clear to
> me how the decrement handles the "free token" concept of the job server
> implementation... also, it's not a good idea to use fputs() in a signal
> handler, and I haven't traced down what other possible issues the other
> calls in increase_job_signal_handler() might have); it doesn't have
> testing with it to make sure it continues to work, and while useful in
> specific situations this feature likely won't be widely needed and so
> get less testing.  And it is limited to only working on POSIX systems
> as others don't support SIGUSRx (IIRC).  I get that we already use
> SIGUSR1 for debug toggling so there is precedent.
> 
> When I realize I started make with a jobserver value I don't like, I
> typically just kill the make and restart it.
> 
> ___
> Bug-make mailing list
> Bug-make@gnu.org
> https://lists.gnu.org/mailman/listinfo/bug-make

Re: Adjusting jobserver size (was: Re: No follow up on patches to support newer glibc ?)

[Henrik Carlqvist]

Thanks for your feedback!

> On the other hand, it's a complex change (I'm not convinced that your
> implementation is complete: for example, it's not immediately clear to
> me how the decrement handles the "free token" concept of the job server
> implementation...

The variable decrease_jobs is set to 0 unless a SIGUSR1 has been received.

With decrease_jobs set to 0 the code in free_child will behave as without
my patch applied, if we have a job server and jobserver_tokens > 1 a job
will be released to the job server and our own remaining jobserver_tokens
will be decreased by 1.

If SIGUSR1 has been used one or more times decrease_jobs will have a value
bigger than 0. Then the code in free_child will still decrease
jobserver_tokens by 1 but instead of relasing the job back to the job
server the variable decrease_jobs will be decreased by 1. That way the
function free_child will make sure that from now on one less parallell job
is being run by make as was intended by one received SIGUSR1.

> also, it's not a good idea to use fputs() in a signal
> handler, 

I can remove those printouts like "Decreased number of jobs to %d" as they
are not necessary for the functionality, they are only a help to the user
to see what is going on. 

> and I haven't traced down what other possible issues the other
> calls in increase_job_signal_handler() might have); it doesn't have
> testing with it to make sure it continues to work,

If you so wish, I can also remove those initializing jobserver-function
calls from the signal handler. Without those function calls it will no
longer be possible to increase number of jobs unless make initially was
started with the -j flag. The call to jobserver_release could also be
moved out of the signal handler, but that would be at the cost of having
to wait for the next finished job before an extra new job will be spawned.
For that reason I would prefer to keep the call to jobserver_release and
add a big fat comment in (at least the posix version of) jobserver_release
that no non reentrat code might be added to that function.

> And it is limited to only working on POSIX systems as others don't
> support SIGUSRx (IIRC). 

This is true, if you have any idea for a more portable solution I am
willing to rewrite my patch. I am also willing to implement different
solutions for different environments, but I am only able to test on Linux
(posix) myself.

> When I realize I started make with a jobserver value I don't like, I
> typically just kill the make and restart it.

This usually works fine for a normal compile. But make can be used for so
much more than compiling programs. Some processes started by make might
need a long time to finish and restarting them might be costful for
different reasons.

I understand that my patch as it looks now is not going to make it into
the next release. However, as I find its functionality useful I am willing
to put more work into the patch if it will help. I am also willing to
sacrifice the ability to add jobs if make was started without "-j" and any
helpful output from the signal handlers.

regards Henrik

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make

Adjusting jobserver size (was: Re: No follow up on patches to support newer glibc ?)

[Paul Smith]

On Thu, 2018-04-05 at 00:26 +0200, Henrik Carlqvist wrote:
> On Wed, 04 Apr 2018 15:42:51 -0400
> Paul Smith  wrote:
> > It does look like we need to make a new release soon.
> 
> If so, is there anything I can do to get the functionality of my
> contributed patch in bug #51200 into the upcoming new release?

Thanks for the reminder Henrik.  For those interested, the link is:
https://savannah.gnu.org/bugs/index.php?51200

I'll confess I'm on the fence about this.  On the one hand I could
imagine where it would be useful.

On the other hand, it's a complex change (I'm not convinced that your
implementation is complete: for example, it's not immediately clear to
me how the decrement handles the "free token" concept of the job server
implementation... also, it's not a good idea to use fputs() in a signal
handler, and I haven't traced down what other possible issues the other
calls in increase_job_signal_handler() might have); it doesn't have
testing with it to make sure it continues to work, and while useful in
specific situations this feature likely won't be widely needed and so
get less testing.  And it is limited to only working on POSIX systems
as others don't support SIGUSRx (IIRC).  I get that we already use
SIGUSR1 for debug toggling so there is precedent.

When I realize I started make with a jobserver value I don't like, I
typically just kill the make and restart it.

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make