Subject: Re: [bug#32121] [PATCH 1/5] base: Compile CHECKOUT in the fiber.

From: [email-masked] (Ludovic Courtès)
Date: Fri, 13 Jul 2018 13:50:21 +0200

Clément Lassieur <[email-masked]> skribis:

> Ludovic Courtès <[email-masked]> writes:
>
>> Morning!
>>
>> Clément Lassieur <[email-masked]> skribis:
>>
>>> Because it may take time and thus prevent PROCESS-SPECS to run every INTERVAL
>>> seconds.
>>>
>>> * src/cuirass/base.scm (process-specs): move the COMPILE invocation inside
>>> SPAWN-FIBER's thunk.  Add log message.
>>
>> [...]
>>
>>> -               (when compile?
>>> -                 (non-blocking (compile checkout)))
>>> -
>>>                 (spawn-fiber
>>>                  (lambda ()
>>> +                  (when compile?
>>> +                    (log-message "compiling '~a' with commit ~s" name commit)
>>> +                    (non-blocking (compile checkout)))
>>
>> I think this doesn’t bring anything compared to the existing
>> ‘non-blocking’ call.
>> The ‘non-blocking’ procedure evaluates its argument in a separate
>> thread; the calling fiber then “waits” for a message from that thread,
>> which it gets when the computation is over.  The ‘get-message’ is
>> non-blocking though: the calling fiber is simply unscheduled until the
>> message has arrived.
>>
>> Does that make sense?
>
> Well, no :-)
>
> My understanding is that non-blocking is, actually... blocking, because
> get-message is blocking.  (It doesn't block the scheduler because it's
> in another thread, but that's not the problem here.)
>
> What I wanted to fix here is the fact that if the build takes one hour,
> we will block for one hour in the COMPILE call, and process-spec won't
> return for one hour.  If it doesn't return for one hour, that means we
> can't evaluate anything else for all that time.

Oh, I see.  However we have to wait for compilation to complete before
continuing anyway, no?

> With my change, the one-hour call will be in the fiber, which means that
> process-spec can return, and other evaluations can be processed.
>
> But this is untested (because compilation doesn't work IIRC), so I can't
> be sure.

Yeah, what about this plan: let’s forget about this patch, and let’s
remove support for compilation altogether in a future patch.

WDYT?

Ludo’.