[m-rev.] for second review: Add future datatype for concurrent and parallel programming

[Paul Bone]

On Thu, Oct 09, 2014 at 11:01:15AM +1100, Julien Fischer wrote:
>
> Hi Paul,
>
> On Thu, 9 Oct 2014, Paul Bone wrote:
>> new file mode 100644
>> index 0000000..fde780d
>> --- /dev/null
>> +++ b/library/thread.future.m
>> @@ -0,0 +1,282 @@
>> +%-----------------------------------------------------------------------------%
>> +% vim: ft=mercury ts=4 sw=4 et
>> +%-----------------------------------------------------------------------------%
>> +% Copyright (C) 2014 The Mercury Team.
>> +% This file may only be copied under the terms of the GNU Library General
>> +% Public License - see the file COPYING.LIB in the Mercury distribution.
>> +%-----------------------------------------------------------------------------%
>> +%
>> +% File: thread.future.m.
>> +% Authors: pbone.
>> +% Stability: low.
>> +%
>> +% This module defines a future data type for parallel and concurrent
>> +% programming.
>
> I suggest:
>
>    This module defines the data type future/1 which is useful for parallel
>    and concurrent programming.
>
> ...
>

I've re-written the start of the module, could you please re-check it?


% This module defines the data types future_io/1 and future/1 which is
% useful for parallel and concurrent programming.
%     
% A future represents a value that might not exist yet.  A value for a
% future may be provided exactly once, but can be read any number of times.
% In these situations futures can be faster than mvars as their
% implementation is simpler: they need only one semaphore and they can avoid
% using it in some cases.
%   
% There are two kinds of futures:
%     
%   + future(T) is a value that will be evaluated by another thread.  The
%     function future/1 will spawn a new thread to evaluate it's argument
%     whose result can be retrieved later by calling the function wait/1.
%     For example:
%   
%       Future = future(SomeFunction),
%       ... do something in the meantime ...
%       Value = wait(Future).
%
%   + future_io(T) provides more flexibility, allowing the caller to control
%     the creation of the thread that provides it's value.  It can be used
%     as follows:
%
%       First:
%           future(Future, !IO),
%     
%       Then in a separate thread:
%           signal(Future, Value0, !IO),
%   
%       Finally, in the original thread:
%           wait(Future, Value, !IO),
%   
%     This is more flexible because the thread can be used to signal
%     multiple futures or do other things, but it requires the I/O state.


>> +
>> +    % Return the value of the future, potentially blocking until the value
>> +    % is available.
>
> What do you mean by "potentially" blocking?  It *will* block until the value
> is available, won't it?
>

I meant that it won't block if the value is already available, but the
"until the value is available" covers this meaning.  I've fixed it.

>> +
>> +    % future_io/1 represents a value that may not have been computed yet.
>> +    % Future values are usually computed by separate threads (using
>> +    % spawn/3).
>
> s/are usually/are intended to be/

Okay.

>> +    %
>> +    % Technically this is a promise however promise is a reserved word in
>> +    % Mercury.  A promise's value can be provided by different calls to
>> +    % signal on different branches of execution.
>
> I'm not sure what this last paragraph is telling me.

Rewritten:
    % Elsewhere this is known as a promise.  We called it future_io because
    % promise is a reserved word in Mercury.

>> diff --git a/library/thread.semaphore.m b/library/thread.semaphore.m
>> index 46c53e2..14f80e1 100644
>> --- a/library/thread.semaphore.m
>> +++ b/library/thread.semaphore.m
>>
>> -    % Returns a new semaphore `Sem' with its counter initialized to Count.
>> +    % Sem = init(Count) returns a new semaphore `Sem' with its counter
>> +    % initialized to Count.
>>     %
>> -:- impure func semaphore.init(int::in) = (semaphore::uo) is det.
>> +    % This has been renamed to impure_init.
>> +    %
>> +:- impure func init(int::in) = (semaphore::uo) is det.
>> +:- pragma obsolete(init/1).
>
> impure_init needs to be (1) a function and (2) defined in the public interface,
> since users require a way of initialising semaphore valued mutables.

Why?  If the rest of the semaphore library was pure I/O code was this ever
used outside of the standard library?  I'm not if we do need to provide such
a function, at least the deprecation will allow people to continue to use
this and we can find out if it causes a problem for anyone.

> The deprecation of the function semaphore.init/1 should be mentioned in
> the NEWS file.

Okay.

>>     % wait(Sem, !IO) blocks until the counter associated with `Sem'
>>     % becomes greater than 0, whereupon it wakes, decrements the
>>     % counter and returns.
>>     %
>> -:- pred semaphore.wait(semaphore::in, io::di, io::uo) is det.
>> +:- pred wait(semaphore::in, io::di, io::uo) is det.
>>
>>     % try_wait(Sem, Succ, !IO) is the same as wait/3, except that
>>     % instead of blocking, it binds `Succ' to a boolean indicating
>>     % whether the call succeeded in obtaining the semaphore or not.
>>     %
>> -:- pred semaphore.try_wait(semaphore::in, bool::out, io::di, io::uo) is det.
>> +:- pred try_wait(semaphore::in, bool::out, io::di, io::uo) is det.
>>
>>     % signal(Sem, !IO) increments the counter associated with `Sem'
>>     % and if the resulting counter has a value greater than 0, it wakes
>>     % one or more coroutines that are waiting on this semaphore (if
>>     % any).
>
> We should probably settle on one of "thread" or "coroutine".  (Since the latter
> only occurs in two places, it should most likely be the former.)
>

I agree, fixed.

>>
>> +init(Count, Semaphore, !IO) :-
>> +    promise_pure (
>> +        impure impure_init(Count, Semaphore)
>> +    ).
>> +
>> +init(Semaphore, !IO) :-
>> +    init(0, Semaphore, !IO).
>> +
>> +init(Count) = Semaphore :-
>> +    impure impure_init(Count, Semaphore).
>> +
>> +signal(Semaphore, !IO) :-
>> +    promise_pure (
>> +        impure impure_signal(Semaphore)
>> +    ).
>
> Guess what the Mercury compiler does to pure det goals with no outputs,
> such as the body of signal/3 there? ;-)
> When you're done guessing, it should be:
>
>    signal(Semaphore, !IO) :-
>        promise_pure (
>            impure impure_signal(Semaphore),
>            !:IO = !.IO
>        ).
>
>> +wait(Semaphore, !IO) :-
>> +    promise_pure (
>> +        impure impure_wait(Semaphore)
>> +    ).
>
> Ditto.

Argh, I had this error earlier and fixed it, then it regressed following the
previous reviews.  Also I noticed that these goals are only optimised away
in low level C and high level C grades, not Java grades, which is
potentially confusing.

The compiler should probably warn about this.


I've fixed everything else.  Thanks.


-- 
Paul Bone