Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] GC performance change since upgrade to 2.14

On Thursday, June 01, 2017 11:01:44 PM Matthias Sohn wrote:
> I discussed this with Christian and we found that we can
> avoid changing public API
> (except the minor issue that we add some new config
> constants to Config which
> we can ignore).
> 
> So I adapted this change accordingly and moved it to
> stable-4.7. Please review the new patchset
> https://git.eclipse.org/r/#/c/98474/ I can release 4.7.1
> as soon as this change is accepted.

<ick> top posting on a long thread, very hard to follow, so 
forgive me if I am missunderstanding what is happening 
here...

I don't believe this a good enough solution for Gerrit.  If 
I understand what is being proposed, this means that auto-gc 
will still run (in the background), and I suspect that many 
installations to do not want jgit gc to run at all.  My 
personal opinion is that gerrit should not run any auto-gc 
(foreground or background) unless I specifically enable such 
a feature.

What feature did we bump jgit for?  Can we revert jgit to a 
version that does not have auto-gc?

-Martin


> On Wed, May 31, 2017 at 5:49 AM, David Turner 
<novalis@xxxxxxxxxxx> wrote:
> > That's fine with me.
> > 
> > On Wed, 2017-05-31 at 01:06 +0200, Matthias Sohn wrote:
> > > instead of introducing a new method void
> > > collectGarbage() in
> > > https://git.eclipse.org/r/#/c/90675/11/org.eclipse.jg
> > > it/src/org/eclip se/jgit/internal/storage/file/GC.java
> > > we could change the existing method
> > > Collection<PackFile> gc() in 4.7.1 to either return
> > > null or an empty collection
> > > in case the garbage collection is run asynchronously.
> > > This way we could avoid
> > > changing API in this service release. In 4.8 we could
> > > add a second method returning a
> > > Future<Collection<PackFile>>.
> > > 
> > > What do you think ?
> > > 
> > > -Matthias
> > > 
> > > On Tue, May 30, 2017 at 5:52 PM, Dave Borowitz
> > > <dborowitz@xxxxxxxxxx>> > 
> > > wrote:
> > > > I think setting a git config option in every repo on
> > > > disk after updating to 2.14 is kind of asking a
> > > > lot, even if we document it. (But then again I
> > > > don't run a Gerrit installation that's affected by
> > > > this issue.)
> > > > 
> > > > On Tue, May 30, 2017 at 11:47 AM, Will Saxon
> > > > <saxonww@xxxxxxxxx>> > > 
> > > > wrote:
> > > > > Is autogc important enough to do this? I'm
> > > > > honestly not sure I'd enable it again even if it
> > > > > worked as intended - we've been using Gerrit for
> > > > > years without it.
> > > > > 
> > > > > On Tue, May 30, 2017 at 10:56 AM, Dave Borowitz
> > > > > <dborowitz@google> > > > 
> > > > > .com> wrote:
> > > > > > On Tue, May 30, 2017 at 10:49 AM, Matthias Sohn
> > > > > > <matthias.sohn@> > > > > 
> > > > > > gmail.com> wrote:
> > > > > > > On Mon, May 29, 2017 at 2:57 PM,
> > > > > > > thomasmulhall410 via Repo> > > > > > 
> > > > > > > and Gerrit Discussion <repo-
discuss@xxxxxxxxxxxxxxxx> wrote:
> > > > > > > > Can't it be backported to 4.7?
> > > > > > > 
> > > > > > > no, it adds new API which is something we
> > > > > > > don't do in service releases
> > > > > > 
> > > > > > If it's absolutely essential that we get this in
> > > > > > the 2.14
> > > > > > branch, can we backport the fix into a custom
> > > > > > JGit build?
> > > > > > 
> > > > > > I mean, I know we can do this technically,
> > > > > > Gerrit master
> > > > > > frequently uses a custom non-release build of
> > > > > > JGit hosted in one of Google's GCS buckets. Up
> > > > > > to release managers whether we would want to
> > > > > > take this step.
> > > > > > 
> > > > > > > > On Monday, May 29, 2017 at 2:01:08 AM UTC+1,
> > > > > > > > David
> > > > > > > > 
> > > > > > > > Pursehouse wrote:
> > > > > > > > > On Mon, May 29, 2017 at 6:57 AM Matthias
> > > > > > > > > Sohn <matthi...@
> > > > > > > > > 
> > > > > > > > > gmail.com> wrote:
> > > > > > > > > > On Fri, May 26, 2017 at 11:09 PM, Luca
> > > > > > > > > > Milanesio <luca.
> > > > > > > > > > 
> > > > > > > > > > mi...@xxxxxxxxx> wrote:
> > > > > > > > > > > Hi Will,
> > > > > > > > > > > v2.14.1 will arrive shortly: a huge
> > > > > > > > > > > number of fixes
> > > > > > > > > > > have been made on top of v2.14 (over
> > > > > > > > > > > 70).
> > > > > > > > > > > 
> > > > > > > > > > > > On 26 May 2017, at 20:58, Will Saxon
> > > > > > > > > > > > <sax...@gmail.
> > > > > > > > > > > > com> wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > Hello,
> > > > > > > > > > > > 
> > > > > > > > > > > > We upgraded to 2.14 last weekend,
> > > > > > > > > > > > and shortly
> > > > > > > > > > > > afterwards started seeing a few
> > > > > > > > > > > > performance issues:
> > > > > > > > > > > > 
> > > > > > > > > > > > - Users reported pushes would never
> > > > > > > > > > > > complete
> > > > > > > > > > > > (traced to autogc)
> > > > > > > > > > > 
> > > > > > > > > > > Auto-GC problem. Just disable it on
> > > > > > > > > > > JGit
> > > > > > > > > > > (see discussion at
> > > > > > > > > > > https://groups.google.com/forum/#!
> > > > > > > > > > > searchin/repo-
> > > > > > > > > > > discuss/autogc$20gerrit$202.14%7Csort:
> > > > > > > > > > > relevance/repo-
> > > > > > > > > > > discuss/y_aO6CvsDvM/dQ9U-IUOAwAJ).
> > > > > > > > > >  
> > > > > > > > > >  fix is underway:
> > > > > > > > > >  https://git.eclipse.org/r/#/c/90675/
> > > > > > > > > 
> > > > > > > > > That fix is going to be in JGit 4.8.0
> > > > > > > > > which I don't think
> > > > > > > > > we'll be able to upgrade to for gerrit
> > > > > > > > > 2.14.x;  Instead
> > > > > > > > > we should mention this known issue and
> > > > > > > > > workaround in the
> > > > > > > > > release notes.
> > > > > > > > > 
> > > > > > > > > > > > - Nightly 'gerrit gc' job began
> > > > > > > > > > > > failing (traced to
> > > > > > > > > > > > NIO2 sshd timeout)
> > > > > > > > > > > 
> > > > > > > > > > > Known issue, fixed in v2.14.1.
> > > > > > > > > > > 
> > > > > > > > > > > > - Nightly builds began timing out
> > > > > > > > > > > > (lack of GC)
> > > > > > > > > > > 
> > > > > > > > > > > Consequence of previous issue.
> > > > > > > > > > > 
> > > > > > > > > > > > We've resolved or worked around all
> > > > > > > > > > > > of the above,
> > > > > > > > > > > > but the last issue we're seeing is
> > > > > > > > > > > > that GC is
> > > > > > > > > > > > taking 3-4x longer than with the
> > > > > > > > > > > > prior version
> > > > > > > > > > > > (2.13.7). We have 228 projects of
> > > > > > > > > > > > varying size, and
> > > > > > > > > > > > prior to the upgrade a 'gerrit gc
> > > > > > > > > > > > --all' took 20-30
> > > > > > > > > > > > minutes. Now it takes ~90 minutes.
> > > > > > > > > > > > Is this
> > > > > > > > > > > > expected?.
> > > > > > > > > > > 
> > > > > > > > > > > Possibly question for Matthias and the
> > > > > > > > > > > JGit mailing
> > > > > > > > > > > list?
> > > > > > > > > > 
> > > > > > > > > > I didn't expect this, needs to be
> > > > > > > > > > investigated.
> > > > > > > > > > JGit gc now moves garbage to loose
> > > > > > > > > > objects to keep
> > > > > > > > > > track of their timestamps individually
> > > > > > > > > > like native git
> > > > > > > > > > does, this could slow
> > > > > > > > > > down gc in case a lot of garbage has to
> > > > > > > > > > be loosened.
> > > > > > > > > > 
> > > > > > > > > > > > On one of our busier projects I ran
> > > > > > > > > > > > a 'gerrit gc
> > > > > > > > > > > > <project> --show-progress' and it
> > > > > > > > > > > > appeared to hang
> > > > > > > > > > > > after the first 'Selecting commits'
> > > > > > > > > > > > step. The
> > > > > > > > > > > > gerrit server was using 100% of 1
> > > > > > > > > > > > CPU core at this
> > > > > > > > > > > > point; running jstack several times
> > > > > > > > > > > > showed that the
> > > > > > > > > > > > GC thread was busy
> > > > > > > > > > > > in
> > > > > > > > > > > > org.eclipse.jgit.internal.storage.p
> > > > > > > > > > > > ack.PackWrite
> > > > > > > > > > > > rBitmapPreparer.setupTipCommitBitmap
> > > > > > > > > > > > s. This phase
> > > > > > > > > > > > of the GC took over 30 minutes by
> > > > > > > > > > > > itself, after
> > > > > > > > > > > > which time the rest of the expected
> > > > > > > > > > > > progress output
> > > > > > > > > > > > was printed.
> > > > > > > > > > > > 
> > > > > > > > > > > > Just wondering if there's some new
> > > > > > > > > > > > configuration
> > > > > > > > > > > > defaults we should look at tuning,
> > > > > > > > > > > > if something is
> > > > > > > > > > > > probably wrong with our
> > > > > > > > > > > > installation, or if maybe
> > > > > > > > > > > > this is expected because GC is doing
> > > > > > > > > > > > more than it
> > > > > > > > > > > > used to. It looks like there have
> > > > > > > > > > > > been a lot of GC-
> > > > > > > > > > > > related changes in JGit 4.6/4.7,
> > > > > > > > > > > > although it's not
> > > > > > > > > > > > obvious from the release notes that
> > > > > > > > > > > > GC performance
> > > > > > > > > > > > would be noticeably different.
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > 
> > > > > > > > > > > > -Will
-- 
The Qualcomm Innovation Center, Inc. is a member of Code 
Aurora Forum, hosted by The Linux Foundation



Back to the top