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
