Re: [gomp4.1] Doacross library implementation

[Aldy Hernandez <aldyh@redhat.com>]

On 10/08/2015 05:48 AM, Torvald Riegel wrote:
> On Thu, 2015-09-24 at 20:32 +0200, Jakub Jelinek wrote:
>> Torvald, can you please have a look at it, if I got all the atomics / memory
>> models right?
>
> More detailed comments below, but in general, I'd really suggest to add
> more code comments for the synchronization parts.  In the end, the level
> of detail of documentation of libgomp is your decision, but, for
> example, the lack of comments in synchronization code in glibc has made
> maintaining this code and fixing issues in it very costly.  It has also
> been hard to understand for many.
>
> My suggestion would be both to (1) document the high-level, abstract
> synchronization scheme and (2) how that scheme is implemented.  The
> first point is important in my experience because typically, the
> high-level scheme and the actual thinking behind it (or, IOW, the intent
> of the original author) is much harder to reconstruct in case of
> concurrent code than it is for sequential code; you can't just simply
> follow the program along line by line, but have to consider
> interleavings.

I couldn't agree more.  After having spent the last month trying to make 
sense of libgomp/task.c, I can honestly say that we need better internal 
documentation.  I know this isn't Jakub's fault, as Richard started the 
non-documenting party, but clearly defined descriptions, functions, and 
implementation go a long way.  APIs and abstractions also make things a 
_lot_ easier to follow.

It could also be that I'm very new to runtime work, specifically 
parallel runtime work, but it was hard to understand.  I think I finally 
have a firm grasp on it (I hope), but it did take me until early this 
week.  Consequently, I took it upon myself to documenting big pieces of 
task.c this week.  I assume anyone not jakub/rth coming after me will 
benefit from it.  So yeah, my upcoming patch will have some variables 
renamed, many more functions with better descriptions (or descriptions 
at all, etc), and a clearly defined API.

Maybe my brain is small; but this stuff is hard.  Every little bit helps :).

p.s. Ironically, it seems that the longer I spend looking at this code, 
the less I feel I need to comment because things are now "obvious", 
which perhaps is an indication that either putting newbies on the 
projects is a good thing, or documenting things early is good practice.

Aldy