git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joshua Jensen <jjensen@workspacewhiz.com>
To: kusmabite@gmail.com
Cc: "Jonathan Nieder" <jrnieder@gmail.com>,
	"Johannes Sixt" <j6t@kdbg.org>, "Thomas Adam" <thomas@xteddy.org>,
	"Ævar Arnfjörð" <avarab@gmail.com>,
	git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Brandon Casey" <drafnel@gmail.com>
Subject: Re: [PATCH/RFC v3 6/8] Add case insensitivity support when using git ls-files
Date: Mon, 04 Oct 2010 08:58:42 -0600	[thread overview]
Message-ID: <4CA9EBA2.9020401@workspacewhiz.com> (raw)
In-Reply-To: <AANLkTikgLzczp1Gkmcg2v35oE2bKxBtxY389Z76FJDRz@mail.gmail.com>

  ----- Original Message -----
From: Erik Faye-Lund
Date: 10/4/2010 8:03 AM
> On Mon, Oct 4, 2010 at 9:49 AM, Jonathan Nieder<jrnieder@gmail.com>  wrote:
>> Johannes Sixt wrote:
>>> On Sonntag, 3. Oktober 2010, Thomas Adam wrote:
>>>> It's a real shame about the code duplication here.  Can we not avoid
>>>> it just by doing:
>>>>
>>>> unsigned char c1 = (ignore_case) ? tolower(*match) : *match;
>>>> unisgned char c2 = (ignore_case) ? tolower(*name) : *name;
>>>>
>>>> I appreciate that to some it might look like perl golf, but...
>>> It has been discussed, and IIRC, the concensus was to keep the code
>>> duplication because this is an inner loop.
>> Did anyone time it?  If it really is not dwarfed by other computation,
>> then how about (warning: ugly!)
>>
> I believe it was timed. I was the one who reacted on this the first
> time around, and I seem to remember that the performance impact was
> indeed significant. This function is used all the time when updating
> the index etc IIRC.
In a good sized repository I have in front of me now, running 'git 
ls-files' through this code path results in 705,374 characters being 
processed by this body of code.  Given the code listed above, that means 
we add 1,410,748 additional comparisons that everyone has to suffer 
through, even those on a case sensitive file system.  Sure, the code 
could be optimized to not perform the double comparison, and the 
compiler may actually perform that optimization.  Still, it is hundreds 
of thousands of additional comparisons and branches that were not there 
before.

I'm running on a really, really fast machine, a Xeon X5560.  The 
difference in time for the above code versus what is in the patch seems 
to average about 0.07 seconds.  Remember, this is an incredibly fast 
machine, and I imagine it will be worse on machines with slower 
processors and less cache.

As discussed in the original thread (which, I believe, was on the 
msysGit mailing list), one of Git's features is its speed.  Maintaining 
that speed in the core.ignorecase=false case is top priority for me, but 
others with more know how can tell me I'm wrong.

Josh

  reply	other threads:[~2010-10-04 14:58 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-03  4:32 [PATCH v2 0/6] Extensions of core.ignorecase=true support Joshua Jensen
2010-10-03  4:32 ` [PATCH v2 1/6] Add string comparison functions that respect the ignore_case variable Joshua Jensen
2010-10-03  8:30   ` Ævar Arnfjörð Bjarmason
2010-10-03  9:07     ` Joshua Jensen
2010-10-03  9:56       ` [PATCH/RFC v3 0/8] ab/icase-directory: jj/icase-directory with Makefile + configure checks Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 1/8] Makefile & configure: add a NO_FNMATCH flag Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 2/8] Makefile & configure: add a NO_FNMATCH_CASEFOLD flag Ævar Arnfjörð Bjarmason
2010-10-03 17:58         ` Johannes Sixt
2010-10-04  2:48           ` [PATCH/RFC v4 " Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 3/8] Add string comparison functions that respect the ignore_case variable Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 4/8] Case insensitivity support for .gitignore via core.ignorecase Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 5/8] Add case insensitivity support for directories when using git status Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 6/8] Add case insensitivity support when using git ls-files Ævar Arnfjörð Bjarmason
2010-10-03 11:54         ` Thomas Adam
2010-10-03 18:19           ` Johannes Sixt
2010-10-03 21:59             ` Thomas Adam
2010-10-04  7:49             ` Jonathan Nieder
2010-10-04  8:02               ` Ævar Arnfjörð Bjarmason
2010-10-04 14:03               ` Erik Faye-Lund
2010-10-04 14:58                 ` Joshua Jensen [this message]
2010-10-04 17:03                   ` Jonathan Nieder
2010-10-04 16:02         ` Robin Rosenberg
2010-10-04 16:41           ` Ævar Arnfjörð Bjarmason
2010-10-04 16:48           ` Erik Faye-Lund
2010-10-04 16:49           ` Joshua Jensen
2010-10-04 17:08             ` Jonathan Nieder
2010-10-04 17:53             ` Ævar Arnfjörð Bjarmason
2010-10-04 19:02           ` Johannes Sixt
2010-10-04 19:17             ` Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 7/8] Support case folding for git add when core.ignorecase=true Ævar Arnfjörð Bjarmason
2010-10-03  9:56       ` [PATCH/RFC v3 8/8] Support case folding in git fast-import " Ævar Arnfjörð Bjarmason
2010-10-07  4:13       ` [PATCH v2 1/6] Add string comparison functions that respect the ignore_case variable Junio C Hamano
2010-10-07  5:48         ` Joshua Jensen
2010-10-03  4:32 ` [PATCH v2 2/6] Case insensitivity support for .gitignore via core.ignorecase Joshua Jensen
2010-10-03  4:32 ` [PATCH v2 3/6] Add case insensitivity support for directories when using git status Joshua Jensen
2010-10-03  4:32 ` [PATCH v2 4/6] Add case insensitivity support when using git ls-files Joshua Jensen
2010-10-03  4:32 ` [PATCH v2 5/6] Support case folding for git add when core.ignorecase=true Joshua Jensen
2010-10-03  4:32 ` [PATCH v2 6/6] Support case folding in git fast-import " Joshua Jensen
2010-10-03 13:00   ` Sverre Rabbelier
2010-10-03  8:17 ` [PATCH v2 0/6] Extensions of core.ignorecase=true support Johannes Sixt
2010-10-03 23:34   ` Junio C Hamano
2010-10-03 11:48 ` Robert Buck
2010-10-03 18:12   ` Johannes Sixt
2010-10-06 22:04     ` Robert Buck
2010-10-06 22:46       ` Joshua Jensen

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=4CA9EBA2.9020401@workspacewhiz.com \
    --to=jjensen@workspacewhiz.com \
    --cc=avarab@gmail.com \
    --cc=drafnel@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jrnieder@gmail.com \
    --cc=kusmabite@gmail.com \
    --cc=thomas@xteddy.org \
    /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).