All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Peart <peartben@gmail.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Git Mailing List <git@vger.kernel.org>,
	Ben Peart <benpeart@microsoft.com>
Subject: Re: [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet
Date: Thu, 18 Oct 2018 15:03:32 -0400	[thread overview]
Message-ID: <ce5077bd-fefc-d415-7237-a2f9a26e3294@gmail.com> (raw)
In-Reply-To: <CACsJy8CvvZcQdxnZbu-FZmVm7wtMDLocjiMVURhvJ=NtuYgi9w@mail.gmail.com>



On 10/18/2018 2:26 PM, Duy Nguyen wrote:
> On Thu, Oct 18, 2018 at 8:18 PM Ben Peart <peartben@gmail.com> wrote:
>> I actually started my effort to speed up reset by attempting to
>> multi-thread refresh_index().  You can see a work in progress at:
>>
>> https://github.com/benpeart/git/pull/new/refresh-index-multithread-gvfs
>>
>> The patch doesn't always work as it is still not thread safe.  When it
>> works, it's great but I ran into to many difficulties trying to debug
>> the remaining threading issues (even adding print statements would
>> change the timing and the repro would disappear).  It will take a lot of
>> code review to discover and fix the remaining non-thread safe code paths.
>>
>> In addition, the optimized code path that takes advantage of fsmonitor,
>> uses multiple threads, fscache, etc _already exists_ in preload_index().
>>    Trying to recreate all those optimizations in refresh_index() is (as I
>> discovered) a daunting task.
> 
> Why not make refresh_index() run preload_index() first (or the
> parallel lstat part to be precise), and only do the heavy
> content-based refresh in single thread mode?
> 

Head smack! Why didn't I think of that?

That is a terrific suggestion.  Calling preload_index() right before the 
big for loop in refresh_index() is a trivial and effective way to do the 
bulk of the updating with the optimized code.  After doing that, most of 
the cache entries can bail out quickly down in refresh_cache_ent() when 
it tests ce_uptodate(ce).

Here are the numbers using that optimization (hot cache, averaged across 
3 runs):

0.32 git add asdf
1.67 git reset asdf
1.68 git status
3.67 Total

vs without it:

0.32 git add asdf
2.48 git reset asdf
1.50 git status
4.30 Total

For a savings in the reset command of 32% and 15% overall.

Clearly doing the refresh_index() faster is not as much savings as not 
doing it at all.  Given how simple this patch is, I think it makes sense 
to do both so that we have optimized each path to is fullest.

  reply	other threads:[~2018-10-18 19:03 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-17 16:40 [PATCH v1 0/2] speed up git reset Ben Peart
2018-10-17 16:40 ` [PATCH v1 1/2] reset: don't compute unstaged changes after reset when --quiet Ben Peart
2018-10-17 18:14   ` Eric Sunshine
2018-10-17 18:22     ` Jeff King
2018-10-18  3:40       ` Junio C Hamano
2018-10-18  6:36         ` Jeff King
2018-10-18 18:15           ` Ben Peart
2018-10-18 18:26             ` Duy Nguyen
2018-10-18 19:03               ` Ben Peart [this message]
2018-10-19  0:34             ` Junio C Hamano
2018-10-17 16:40 ` [PATCH v1 2/2] reset: add new reset.quietDefault config setting Ben Peart
2018-10-17 18:19   ` Eric Sunshine
2018-10-17 18:23     ` Jeff King
2018-10-23  9:13       ` Ævar Arnfjörð Bjarmason
2018-10-23 18:11         ` Ben Peart
2018-10-23 20:02           ` Jeff King
2018-10-23 20:03           ` Ævar Arnfjörð Bjarmason
2018-10-24 15:48             ` Recommended configurations (was Re: [PATCH v1 2/2] reset: add new reset.quietDefault config setting) Derrick Stolee
2018-10-24 23:58               ` Jeff King
2018-10-25  4:09                 ` Junio C Hamano
2018-10-19 16:12 ` [PATCH v2 0/3] speed up git reset Ben Peart
2018-10-19 16:12   ` [PATCH v2 1/3] reset: don't compute unstaged changes after reset when --quiet Ben Peart
2018-10-19 16:12   ` [PATCH v2 2/3] reset: add new reset.quiet config setting Ben Peart
2018-10-19 16:36     ` Eric Sunshine
2018-10-19 16:46       ` Jeff King
2018-10-19 17:10         ` Eric Sunshine
2018-10-19 17:11           ` Jeff King
2018-10-19 17:23             ` Ben Peart
2018-10-19 19:08               ` Jeff King
2018-10-22  5:04               ` Junio C Hamano
2018-10-19 17:11         ` Ben Peart
2018-10-19 16:12   ` [PATCH v2 3/3] reset: warn when refresh_index() takes more than 2 seconds Ben Peart
2018-10-22 13:18 ` [PATCH v3 0/3] speed up git reset Ben Peart
2018-10-22 13:18   ` [PATCH v3 1/3] reset: don't compute unstaged changes after reset when --quiet Ben Peart
2018-10-22 20:44     ` Johannes Schindelin
2018-10-22 22:07       ` Ben Peart
2018-10-23  8:53         ` Johannes Schindelin
2018-10-23 15:46         ` Duy Nguyen
2018-10-23 19:55           ` Johannes Schindelin
2018-10-22 13:18   ` [PATCH v3 2/3] reset: add new reset.quiet config setting Ben Peart
2018-10-22 14:45     ` Duy Nguyen
2018-10-23 18:47       ` Ben Peart
2018-10-24  2:56         ` Junio C Hamano
2018-10-24  7:21           ` Junio C Hamano
2018-10-24 14:54           ` Duy Nguyen
2018-10-25  1:12             ` Junio C Hamano
2018-10-24 14:49         ` Duy Nguyen
2018-10-22 19:13     ` Ramsay Jones
2018-10-22 20:06       ` Jeff King
2018-10-23 17:31         ` Ben Peart
2018-10-23 17:35           ` Jeff King
2018-10-22 13:18   ` [PATCH v3 3/3] reset: warn when refresh_index() takes more than 2 seconds Ben Peart
2018-10-23  0:23     ` Junio C Hamano
2018-10-23 17:12       ` Ben Peart
2018-10-23 19:04 ` [PATCH v4 0/3] speed up git reset Ben Peart
2018-10-23 19:04   ` [PATCH v4 1/3] reset: don't compute unstaged changes after reset when --quiet Ben Peart
2018-10-23 19:04   ` [PATCH v4 2/3] reset: add new reset.quiet config setting Ben Peart
2018-10-24  0:39     ` Ramsay Jones
2018-10-25  4:56       ` Junio C Hamano
2018-10-25  9:26         ` Junio C Hamano
2018-10-25 13:26           ` Ben Peart
2018-10-25 17:04           ` Ramsay Jones
2018-10-23 19:04   ` [PATCH v4 3/3] reset: warn when refresh_index() takes more than 2 seconds Ben Peart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ce5077bd-fefc-d415-7237-a2f9a26e3294@gmail.com \
    --to=peartben@gmail.com \
    --cc=benpeart@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.