This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GCC Steering Committee attention] [PING] [PING] [PING] libgomp: In OpenACC testing, cycle though $offload_targets, and by default only build for the offload target that we're actually going to test


On 5 August 2016 at 15:06, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Thu, Aug 04, 2016 at 09:12:36PM +0100, Manuel López-Ibáñez wrote:
>> This is a problem throughout GCC. We have a single C++ maintainer, a
>> single part-time C maintainer, none? for libiberty, no regular
>> maintainer for build machinery, and so on and so forth.
>
> I'd object to this being "throughout" GCC. There are a number of very good,
> very responsive, very helpful GCC reviewers in our community. However, as
> you'd expect, the most active areas for contribution are the most active
> areas for review.

Both statements are not conflicting. It may be true at the same time
that reviewer X is very good, very responsive, very helpful, skilled,
and experienced,  and also true that patches for the areas maintained
by reviewer X may still not be reviewed timely. And, in case it needs
repeating, the solution is not that X stops reviewing or maintaining.

>> This has been a problem for a very long time, but it seems to be
>> getting worse now that fewer contributors are doing most of the
>> work.
>
> Rumours of GCC's death have been greatly exagerated!
>
> At times it might feel this way, but the data
> (BlackDuck at www.openhub.net and my own analysis on the Git Mirror)
> suggests that actually the GCC community is near enough as strong as it
> has been for most of the past 20 years, both in terms of volume of
> contributions, number of contributors, and average number of contributions
> per contributor. Some rudimentary stats gathering on the git mirror suggests
> no substantial change in the makeup of the community over the years. If
> anything there are more commits from more people and the average number of
> commits per committer is decreasing.

I think those conclusions are debatable:

* GCC has also grown over the years, there is a lot more code and
areas, specially more targets, which attract their own temporary
developers who do not contribute to the rest of the compiler (much
less review patches for the rest of the compiler).

* Your analysis includes Ada, Go and Fortran. I'd suggest to exclude
them, since in terms of developers and reviewing, they seem to be
doing fine. They also tend to organise themselves mostly independently
of the rest of the compiler. This is also mostly true for targets.

* 100 commits is less than 2%. Quite a low threshold. Perhaps 1%, 25%,
50%, 75%, 90% are more informative.

* https://www.openhub.net/p/taezaza/contributors/summary shows that
more than 25% of the commits in the last 12 months were made by 6
people. Note that those people are also the most active reviewers.

* If I adjust the numbers by the total number of contributors, then we
get a different picture:

                       X1998       X2001       X2004       X2007
X2010       X2013       X2015       X2016
0-19 commits       36.363636   47.413793   46.405229   54.491018
60.233918   62.500000   60.526316   66.447368
20-39 commits      18.181818   16.379310   16.993464   15.568862
16.374269   15.909091   16.842105   14.473684
40-59 commits       9.090909    7.758621    6.535948   11.377246
7.602339    9.090909    6.315789    8.552632
60-79 commits       6.818182    6.034483    6.535948    5.389222
1.754386    2.272727    2.631579    1.973684
80-99 commits       4.545455    5.172414    6.535948    2.994012
2.339181    2.272727    2.105263    3.289474
100+ commits       25.000000   18.103448   17.647059   10.778443
12.280702    8.522727   12.105263    5.921053

that is, most of the commits are done by smaller fraction of the
total. If that smaller percentage is also responsible for doing most
of the review work for the rest...

* Numbers for other years might shed more light. 2010, 2013 and 2015
might have been especial in one sense or another.

>> >Global Reviewers are welcome to review OpenACC/OpenMP/offloading patches.
>> >But that doesn't help if that's then not happening in reality.  (With the
>> >exception of Bernd, who then did review such patches for a while, but
>> >also seems to have stopped with that again.)
>>
>> At least half of the global reviewers in MAINTAINERS never review
>> any patches. Most of them are not active any more and presumably do
>> not read GCC emails.
>
> That's just false.

Sorry, this was poorly phrased. I meant most of the half that do not
do global reviews. Still, I should have phrased that better by saying
that it was my biased perception. I tend to only pay attention to FE
and diagnostic patches and...

> I've seen all of these active on the lists during 2016 to a lesser or
> greater exentent.

Actively reviewing patches outside their maintainership area? For
example, reviewing OpenACC/OpenMP/offloading patches? Again, this is
not a criticism of people. My point is that we cannot extrapolate that
there are enough reviewers by claiming that there are N global
reviewers in MAINTAINERS. Or as Thomas more eloquently puts it: "that
doesn't help if that's then not happening in reality."

That said, I'm ok with conceding that I flunked my numbers and
dropping this point. It doesn't seem it will lead us to anywhere
useful.


>> a) Follow LLVM's model and allow changes to be reviewed after commit.
>
> If we were to take anything from LLVM's model it should be the use of a
> review tool that makes prioritising patches to review by age easier.
> I love email, but it is a stone-age tool for patch review and makes it
> very easy for patches to fall through the cracks.

Fully agreed. But such tool will only be accepted if it makes life not
harder for the people actually reviewing and makes life easy enough
that they are motivated to use it, and we have not found such tool.
The old patch tracker had patches languishing there for ages. There is
no reason to think that a different tool will result in a different
outcome.

> The commit/revert/commit/revert/commit/revert/commit model doesn't look
> particularly helpful (though it does help your blackduck numbers I guess)
> and *requires* effective continuous integration testing infrastructure across
> targets. Only by being able to quickly identify and revert a broken commit
> do you remove the pain. Otherwise you've shifted trouble from one person
> waiting for a review to N maintainers manually bisecting to find out why
> their target has regressed. That's not a good way for any of us reviewers
> and maintainers to spend our time.

We do not require most patches to be build or tested on several
targets. And a maintainer could still ask for more testing before a
commit without spending time on a full review. Thus, I don't think the
situation that you describe is more likely than the usual churn that
happens already. I might be wrong, I don't pay much attention to
back-end patches to be honest and those may get revised by target
maintainers before being committed, more than in other areas.

> If someone wants to build some good scripts such that we could use to
> build continuous integration testing infrastructure, that would probably
> be very valuable to the community. Things like robots that repeatedly build
> and then bisect bootstrap failures on the Compile Farm machines would be
> handy. Better yet if you can automatically pick up testsuite regressions.

This seems to do at least half of what you want:
http://toolchain.lug-owl.de/buildbot/

>> d) Delegate hierarchically. Module owners should seek and delegate
>> to people with svn-write powers and ask for reviews in exchange of
>> reviews.
>
> This is current policy, see the section "Reviewers" section in MAINTAINERS.

Perhaps I was not clear here. What I mean is that maintainers would
act like editors in scientific journals, they will reject obviously
wrong patches and find a reviewer of their choosing that would accept
to review non-obvious patches whenever they do not have time or
interest in reviewing the patches themselves.

Thanks, your comments were insightful. If more people give feedback
and there is further interest, I will summarize the Pros/Cons/comments
in the wiki.

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]