diff mbox series

[v3,1/3] reset: don't compute unstaged changes after reset when --quiet

Message ID 20181022131828.21348-2-peartben@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] reset: don't compute unstaged changes after reset when --quiet | expand

Commit Message

Ben Peart Oct. 22, 2018, 1:18 p.m. UTC 
From: Ben Peart <benpeart@microsoft.com>

When git reset is run with the --quiet flag, don't bother finding any
additional unstaged changes as they won't be output anyway.  This speeds up
the git reset command by avoiding having to lstat() every file looking for
changes that aren't going to be reported anyway.

The savings can be significant.  In a repo with 200K files "git reset"
drops from 7.16 seconds to 0.32 seconds for a savings of 96%.

Signed-off-by: Ben Peart <benpeart@microsoft.com>
---
 builtin/reset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Johannes Schindelin Oct. 22, 2018, 8:44 p.m. UTC | #1 
Hi Ben,

On Mon, 22 Oct 2018, Ben Peart wrote:

> From: Ben Peart <benpeart@microsoft.com>
> 
> When git reset is run with the --quiet flag, don't bother finding any
> additional unstaged changes as they won't be output anyway.  This speeds up
> the git reset command by avoiding having to lstat() every file looking for
> changes that aren't going to be reported anyway.
> 
> The savings can be significant.  In a repo with 200K files "git reset"
> drops from 7.16 seconds to 0.32 seconds for a savings of 96%.

That's very nice!

Those numbers, just out of curiosity, are they on Windows? Or on Linux?

Ciao,
Dscho
Ben Peart Oct. 22, 2018, 10:07 p.m. UTC | #2 
> -----Original Message-----
> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Sent: Monday, October 22, 2018 4:45 PM
> To: Ben Peart <peartben@gmail.com>
> Cc: git@vger.kernel.org; gitster@pobox.com; Ben Peart
> <Ben.Peart@microsoft.com>; peff@peff.net; sunshine@sunshineco.com
> Subject: Re: [PATCH v3 1/3] reset: don't compute unstaged changes after
> reset when --quiet
> 
> Hi Ben,
> 
> On Mon, 22 Oct 2018, Ben Peart wrote:
> 
> > From: Ben Peart <benpeart@microsoft.com>
> >
> > When git reset is run with the --quiet flag, don't bother finding any
> > additional unstaged changes as they won't be output anyway.  This speeds
> up
> > the git reset command by avoiding having to lstat() every file looking for
> > changes that aren't going to be reported anyway.
> >
> > The savings can be significant.  In a repo with 200K files "git reset"
> > drops from 7.16 seconds to 0.32 seconds for a savings of 96%.
> 
> That's very nice!
> 
> Those numbers, just out of curiosity, are they on Windows? Or on Linux?
> 

It's safe to assume all my numbers are on Windows. :-)

> Ciao,
> Dscho
Johannes Schindelin Oct. 23, 2018, 8:53 a.m. UTC | #3 
Hi Ben,

On Mon, 22 Oct 2018, Ben Peart wrote:

> > From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > 
> > On Mon, 22 Oct 2018, Ben Peart wrote:
> > 
> > > When git reset is run with the --quiet flag, don't bother finding
> > > any additional unstaged changes as they won't be output anyway.
> > > This speeds up the git reset command by avoiding having to lstat()
> > > every file looking for changes that aren't going to be reported
> > > anyway.
> > >
> > > The savings can be significant.  In a repo with 200K files "git
> > > reset" drops from 7.16 seconds to 0.32 seconds for a savings of 96%.
> > 
> > That's very nice!
> > 
> > Those numbers, just out of curiosity, are they on Windows? Or on
> > Linux?
> > 
> 
> It's safe to assume all my numbers are on Windows. :-)

Excellent. These speed-ups will really help our users.

Thanks!
Dscho
Duy Nguyen Oct. 23, 2018, 3:46 p.m. UTC | #4 
On Tue, Oct 23, 2018 at 1:01 AM Ben Peart <Ben.Peart@microsoft.com> wrote:
>
> > -----Original Message-----
> > From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > Sent: Monday, October 22, 2018 4:45 PM
> > To: Ben Peart <peartben@gmail.com>
> > Cc: git@vger.kernel.org; gitster@pobox.com; Ben Peart
> > <Ben.Peart@microsoft.com>; peff@peff.net; sunshine@sunshineco.com
> > Subject: Re: [PATCH v3 1/3] reset: don't compute unstaged changes after
> > reset when --quiet
> >
> > Hi Ben,
> >
> > On Mon, 22 Oct 2018, Ben Peart wrote:
> >
> > > From: Ben Peart <benpeart@microsoft.com>
> > >
> > > When git reset is run with the --quiet flag, don't bother finding any
> > > additional unstaged changes as they won't be output anyway.  This speeds
> > up
> > > the git reset command by avoiding having to lstat() every file looking for
> > > changes that aren't going to be reported anyway.
> > >
> > > The savings can be significant.  In a repo with 200K files "git reset"
> > > drops from 7.16 seconds to 0.32 seconds for a savings of 96%.
> >
> > That's very nice!
> >
> > Those numbers, just out of curiosity, are they on Windows? Or on Linux?
> >
>
> It's safe to assume all my numbers are on Windows. :-)

It does bug me about this. Next time please mention the platform you
tested on in the commit message. Not all platforms behave the same way
especially when it comes to performance.

>
> > Ciao,
> > Dscho
>
>
Johannes Schindelin Oct. 23, 2018, 7:55 p.m. UTC | #5 
Hi Duy,

On Tue, 23 Oct 2018, Duy Nguyen wrote:

> On Tue, Oct 23, 2018 at 1:01 AM Ben Peart <Ben.Peart@microsoft.com> wrote:
> >
> > > -----Original Message-----
> > > From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > > Sent: Monday, October 22, 2018 4:45 PM
> > > To: Ben Peart <peartben@gmail.com>
> > > Cc: git@vger.kernel.org; gitster@pobox.com; Ben Peart
> > > <Ben.Peart@microsoft.com>; peff@peff.net; sunshine@sunshineco.com
> > > Subject: Re: [PATCH v3 1/3] reset: don't compute unstaged changes after
> > > reset when --quiet
> > >
> > > Hi Ben,
> > >
> > > On Mon, 22 Oct 2018, Ben Peart wrote:
> > >
> > > > From: Ben Peart <benpeart@microsoft.com>
> > > >
> > > > When git reset is run with the --quiet flag, don't bother finding any
> > > > additional unstaged changes as they won't be output anyway.  This speeds
> > > up
> > > > the git reset command by avoiding having to lstat() every file looking for
> > > > changes that aren't going to be reported anyway.
> > > >
> > > > The savings can be significant.  In a repo with 200K files "git reset"
> > > > drops from 7.16 seconds to 0.32 seconds for a savings of 96%.
> > >
> > > That's very nice!
> > >
> > > Those numbers, just out of curiosity, are they on Windows? Or on Linux?
> > >
> >
> > It's safe to assume all my numbers are on Windows. :-)
> 
> It does bug me about this. Next time please mention the platform you
> tested on in the commit message. Not all platforms behave the same way
> especially when it comes to performance.

And pretty much all different testing scenarios behave differently, too.
And at some stage, we're asking for too many fries.

In other words: we always accepted performance improvements when it could
be demonstrated that they improved a certain not too uncommon scenario,
and I do not think it would make sense to change this stance now. Not
unless you can demonstrate a good reason why we should.

Ciao,
Johannes
diff mbox series

Patch

diff --git a/builtin/reset.c b/builtin/reset.c
index 11cd0dcb8c..04f0d9b4f5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -375,7 +375,7 @@  int cmd_reset(int argc, const char **argv, const char *prefix)
 			int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;
 			if (read_from_tree(&pathspec, &oid, intent_to_add))
 				return 1;
-			if (get_git_work_tree())
+			if (!quiet && get_git_work_tree())
 				refresh_index(&the_index, flags, NULL, NULL,
 					      _("Unstaged changes after reset:"));
 		} else {