git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Peart <peartben@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, benpeart@microsoft.com, pclouds@gmail.com,
	johannes.schindelin@gmx.de, David.Turner@twosigma.com,
	peff@peff.net, christian.couder@gmail.com, avarab@gmail.com
Subject: Re: [PATCH v5 7/7] fsmonitor: add a performance test
Date: Fri, 7 Jul 2017 15:07:31 -0400	[thread overview]
Message-ID: <bade1166-e646-b05a-f65b-adb8da8ba0a7@gmail.com> (raw)
In-Reply-To: <xmqqeftsayeg.fsf@gitster.mtv.corp.google.com>



On 7/7/2017 2:35 PM, Junio C Hamano wrote:
> Ben Peart <peartben@gmail.com> writes:
> 
>> On 6/14/2017 2:36 PM, Junio C Hamano wrote:
>>> Ben Peart <peartben@gmail.com> writes:
>>>
>>>>> Having said all that, I think you are using this ONLY on windows;
>>>>> perhaps it is better to drop #ifdef GIT_WINDOWS_NATIVE from all of
>>>>> the above and arrange Makefile to build test-drop-cache only on that
>>>>> platform, or something?
>>>>
>>>> I didn't find any other examples of Windows only tools.  I'll update
>>>> the #ifdef to properly dump the file system cache on Linux as well and
>>>> only error out on other platforms.
>>>
>>> If this will become Windows-only, then I have no problem with
>>> platform specfic typedef ;-) I have no problem with CamelCase,
>>> either, as that follows the local convention on the platform
>>> (similar to those in compat/* that are only for Windows).
>>>
>>> Having said all that.
>>>
>>> Another approach is to build this helper on all platforms, ...
> 
> ... and having said all that, I think it is perfectly fine to do
> such a clean-up long after the series gets more exposure to wider
> audiences as a follow-up patch.  Let's get the primary part that
> affects people's everyday use of Git right and then worry about the
> test details later.
> 
> A quick show of hands to the list audiences.  How many of you guys
> actually tried this series on 'pu' and checked to see its
> performance (and correctness ;-) characteristics?

TLDR: the current version isn't correct.

One of the things I did was hack up the test script to enable running 
all the tests with fsmonitor enabled.  I found a number of bugs in 
Watchman on Windows and have been working with Wez to get them fixed.

Just last week Watchman got to the point where I could run the complete 
git test suite.  As a result, I found that fsmonitor is overly 
aggressive in marking things with ce_mark_uptodate.  Submodules are 
currently broken as are commands that pass "--ignore-missing."

I started reworking the logic to fix these bugs and realized I was 
duplicating the set of tests that already exist in preload_thread. I'm 
currently working on integrating the logic into preload_thread so that 
both options can be used in combination and so I can avoid doing the 
(nearly identical) loop twice.

> 
> Do you folks like it?  Rather not have such complexity in the core
> part of the system?  A good first step to start adding more
> performance improvements?  No opinion?
> 
> 

  reply	other threads:[~2017-07-07 19:07 UTC|newest]

Thread overview: 137+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-10 13:40 [PATCH v5 0/7] Fast git status via a file system watcher Ben Peart
2017-06-10 13:40 ` [PATCH v5 1/7] bswap: add 64 bit endianness helper get_be64 Ben Peart
2017-06-10 13:40 ` [PATCH v5 2/7] dir: make lookup_untracked() available outside of dir.c Ben Peart
2017-06-10 13:40 ` [PATCH v5 3/7] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files Ben Peart
2017-06-27 15:43   ` Christian Couder
2017-07-03 21:25     ` Ben Peart
2017-06-10 13:40 ` [PATCH v5 4/7] fsmonitor: add test cases for fsmonitor extension Ben Peart
2017-06-27 16:20   ` Christian Couder
2017-07-07 18:50     ` Ben Peart
2017-06-10 13:40 ` [PATCH v5 5/7] fsmonitor: add documentation for the " Ben Peart
2017-06-10 13:40 ` [PATCH v5 6/7] fsmonitor: add a sample query-fsmonitor hook script for Watchman Ben Peart
2017-06-10 13:40 ` [PATCH v5 7/7] fsmonitor: add a performance test Ben Peart
2017-06-10 14:04   ` Ben Peart
2017-06-12 22:04   ` Junio C Hamano
2017-06-14 14:12     ` Ben Peart
2017-06-14 18:36       ` Junio C Hamano
2017-07-07 18:14         ` Ben Peart
2017-07-07 18:35           ` Junio C Hamano
2017-07-07 19:07             ` Ben Peart [this message]
2017-07-07 19:33             ` David Turner
2017-07-08  7:19             ` Christian Couder
2017-06-28  5:11 ` [PATCH v5 0/7] Fast git status via a file system watcher Christian Couder
2017-07-10 13:36   ` Ben Peart
2017-07-10 14:40     ` Ben Peart
2017-09-15 19:20 ` [PATCH v6 00/12] " Ben Peart
2017-09-15 19:20   ` [PATCH v6 01/12] bswap: add 64 bit endianness helper get_be64 Ben Peart
2017-09-15 19:20   ` [PATCH v6 02/12] preload-index: add override to enable testing preload-index Ben Peart
2017-09-15 19:20   ` [PATCH v6 03/12] update-index: add a new --force-write-index option Ben Peart
2017-09-15 19:20   ` [PATCH v6 04/12] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files Ben Peart
2017-09-15 21:35     ` David Turner
2017-09-18 13:07       ` Ben Peart
2017-09-18 13:32         ` David Turner
2017-09-18 13:49           ` Ben Peart
2017-09-15 19:20   ` [PATCH v6 05/12] fsmonitor: add documentation for the fsmonitor extension Ben Peart
2017-09-15 19:43     ` David Turner
2017-09-18 13:27       ` Ben Peart
2017-09-17  8:03     ` Junio C Hamano
2017-09-18 13:29       ` Ben Peart
2017-09-15 19:20   ` [PATCH v6 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit Ben Peart
2017-09-15 20:34     ` David Turner
2017-09-15 19:20   ` [PATCH v6 07/12] update-index: add fsmonitor support to update-index Ben Peart
2017-09-15 19:20   ` [PATCH v6 08/12] fsmonitor: add a test tool to dump the index extension Ben Peart
2017-09-17  8:02     ` Junio C Hamano
2017-09-18 13:38       ` Ben Peart
2017-09-18 15:43         ` Torsten Bögershausen
2017-09-18 16:28           ` Ben Peart
2017-09-19 14:16             ` Torsten Bögershausen
2017-09-19 15:36               ` Ben Peart
2017-09-15 19:20   ` [PATCH v6 09/12] split-index: disable the fsmonitor extension when running the split index test Ben Peart
2017-09-19 20:43     ` Jonathan Nieder
2017-09-20 17:11       ` Ben Peart
2017-09-20 17:46         ` Jonathan Nieder
2017-09-21  0:05           ` Ben Peart
2017-09-15 19:20   ` [PATCH v6 10/12] fsmonitor: add test cases for fsmonitor extension Ben Peart
2017-09-15 22:00     ` David Turner
2017-09-19 19:32       ` David Turner
2017-09-19 20:30         ` Ben Peart
2017-09-16 15:27     ` Torsten Bögershausen
2017-09-17  5:43       ` [PATCH v1 1/1] test-lint: echo -e (or -E) is not portable tboegi
2017-09-19 20:37         ` Jonathan Nieder
2017-09-20 13:49           ` Torsten Bögershausen
2017-09-22  1:04             ` Junio C Hamano
2017-09-18 14:06       ` [PATCH v6 10/12] fsmonitor: add test cases for fsmonitor extension Ben Peart
2017-09-17  4:47     ` Junio C Hamano
2017-09-18 15:25       ` Ben Peart
2017-09-19 20:34         ` Jonathan Nieder
2017-09-15 19:20   ` [PATCH v6 11/12] fsmonitor: add a sample integration script for Watchman Ben Peart
2017-09-15 19:20   ` [PATCH v6 12/12] fsmonitor: add a performance test Ben Peart
2017-09-15 21:56     ` David Turner
2017-09-18 14:24     ` Johannes Schindelin
2017-09-18 18:19       ` Ben Peart
2017-09-19 15:28         ` Johannes Schindelin
2017-09-19 19:27   ` [PATCH v7 00/12] Fast git status via a file system watcher Ben Peart
2017-09-19 19:27     ` [PATCH v7 01/12] bswap: add 64 bit endianness helper get_be64 Ben Peart
2017-09-19 19:27     ` [PATCH v7 02/12] preload-index: add override to enable testing preload-index Ben Peart
2017-09-20 22:06       ` Stefan Beller
2017-09-21  0:02         ` Ben Peart
2017-09-21  0:44           ` Stefan Beller
2017-09-19 19:27     ` [PATCH v7 03/12] update-index: add a new --force-write-index option Ben Peart
2017-09-20  5:47       ` Junio C Hamano
2017-09-20 14:58         ` Ben Peart
2017-09-21  1:46           ` Junio C Hamano
2017-09-21  2:06             ` Ben Peart
2017-09-21  2:18               ` Junio C Hamano
2017-09-21  2:32                 ` Junio C Hamano
2017-09-19 19:27     ` [PATCH v7 04/12] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files Ben Peart
2017-09-20  2:28       ` Junio C Hamano
2017-09-20 16:19         ` Ben Peart
2017-09-21  2:00           ` Junio C Hamano
2017-09-21  2:24             ` Ben Peart
2017-09-21 14:35               ` Ben Peart
2017-09-22  1:02                 ` Junio C Hamano
2017-09-20  6:23       ` Junio C Hamano
2017-09-20 16:29         ` Ben Peart
2017-09-19 19:27     ` [PATCH v7 05/12] fsmonitor: add documentation for the fsmonitor extension Ben Peart
2017-09-20 10:00       ` Martin Ågren
2017-09-20 17:02         ` Ben Peart
2017-09-20 17:11           ` Martin Ågren
2017-09-19 19:27     ` [PATCH v7 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit Ben Peart
2017-09-19 19:46       ` David Turner
2017-09-19 20:44         ` Ben Peart
2017-09-19 21:27           ` David Turner
2017-09-19 22:44             ` Ben Peart
2017-09-19 19:27     ` [PATCH v7 07/12] update-index: add fsmonitor support to update-index Ben Peart
2017-09-19 19:27     ` [PATCH v7 08/12] fsmonitor: add a test tool to dump the index extension Ben Peart
2017-09-19 19:27     ` [PATCH v7 09/12] split-index: disable the fsmonitor extension when running the split index test Ben Peart
2017-09-19 19:27     ` [PATCH v7 10/12] fsmonitor: add test cases for fsmonitor extension Ben Peart
2017-09-19 19:27     ` [PATCH v7 11/12] fsmonitor: add a sample integration script for Watchman Ben Peart
2017-09-19 19:27     ` [PATCH v7 12/12] fsmonitor: add a performance test Ben Peart
2017-09-22 16:35     ` [PATCH v8 00/12] Fast git status via a file system watcher Ben Peart
2017-09-22 16:35       ` [PATCH v8 01/12] bswap: add 64 bit endianness helper get_be64 Ben Peart
2017-09-22 23:37         ` Martin Ågren
2017-09-23 23:31           ` Ben Peart
2017-09-24  3:51             ` Jeff King
2017-09-24  3:52             ` Junio C Hamano
2017-09-22 16:35       ` [PATCH v8 02/12] preload-index: add override to enable testing preload-index Ben Peart
2017-09-22 16:35       ` [PATCH v8 03/12] update-index: add a new --force-write-index option Ben Peart
2017-09-22 16:35       ` [PATCH v8 04/12] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files Ben Peart
2017-09-22 16:35       ` [PATCH v8 05/12] fsmonitor: add documentation for the fsmonitor extension Ben Peart
2017-09-22 16:35       ` [PATCH v8 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit Ben Peart
2017-09-22 16:35       ` [PATCH v8 07/12] update-index: add fsmonitor support to update-index Ben Peart
2017-09-22 16:35       ` [PATCH v8 08/12] fsmonitor: add a test tool to dump the index extension Ben Peart
2017-09-22 23:37         ` Martin Ågren
2017-09-23 23:33           ` Ben Peart
2017-09-24  3:51             ` Junio C Hamano
2017-09-22 16:35       ` [PATCH v8 09/12] split-index: disable the fsmonitor extension when running the split index test Ben Peart
2017-09-22 16:35       ` [PATCH v8 10/12] fsmonitor: add test cases for fsmonitor extension Ben Peart
2017-09-22 16:35       ` [PATCH v8 11/12] fsmonitor: add a sample integration script for Watchman Ben Peart
2017-09-22 16:35       ` [PATCH v8 12/12] fsmonitor: add a performance test Ben Peart
2017-09-29  2:20       ` [PATCH v8 00/12] Fast git status via a file system watcher Junio C Hamano
2017-09-29 12:07         ` Ben Peart
2017-10-01  8:24           ` Junio C Hamano
2017-10-03 19:48             ` Ben Peart
2017-10-04  2:09               ` Junio C Hamano
2017-10-04  6:38                 ` Alex Vandiver
2017-10-04 12:48                   ` Ben Peart
2017-10-04 12:27                 ` 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=bade1166-e646-b05a-f65b-adb8da8ba0a7@gmail.com \
    --to=peartben@gmail.com \
    --cc=David.Turner@twosigma.com \
    --cc=avarab@gmail.com \
    --cc=benpeart@microsoft.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).