Re: [PATCH] check_refname_component: Optimize

2014-05-31 Thread Michael Haggerty
On 05/30/2014 07:29 PM, Jeff King wrote: On Fri, May 30, 2014 at 11:47:33AM +0200, Michael Haggerty wrote: [...] If we want to be robust to future changes to refname rules, we could add a header flag like # pack-refs with: peeled fully-peeled check-level=1.0 [...] Yeah, I thought about

Re: [PATCH] check_refname_component: Optimize

2014-05-31 Thread Duy Nguyen
On Fri, May 30, 2014 at 7:07 AM, Jeff King p...@peff.net wrote: But then we're just trusting that the trust me flag on disk is correct. Why not just trust that packed-refs is correct in the first place? I missed one thing in the first reply: because packed-refs is a plain text file, the user

Re: [PATCH] check_refname_component: Optimize

2014-05-30 Thread Michael Haggerty
On 05/30/2014 02:07 AM, Jeff King wrote: On Fri, May 30, 2014 at 06:43:18AM +0700, Duy Nguyen wrote: The first time we read packed_refs, check_refname_format() is called in read_packed_refs()-create_ref_entry() as usual. If we find no problem, we store packed_refs stat() info in maybe

Re: [PATCH] check_refname_component: Optimize

2014-05-30 Thread Jeff King
On Fri, May 30, 2014 at 11:47:33AM +0200, Michael Haggerty wrote: I could guess something like the writer has a different idea of what a valid refname is than we do. But that applies as well to (2), but just as the reader who wrote packed-refs.stat has a different idea than we do. If

Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread brian m. carlson
On Wed, May 28, 2014 at 03:57:35PM -0400, David Turner wrote: +ifdef NO_SSE + BASIC_CFLAGS += -DNO_SSE This is not a great name. This implies the system does not have SSE (SSE 1), but in fact what you're concerned about is SSE 4. This will be confusing for users of older x86-64

Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes: On Thu, May 29, 2014 at 6:49 AM, David Turner dtur...@twopensource.com wrote: I assume that most of the time spent in check_refname_component() is while reading the packed-refs file, right? Yes. I wonder if we can get away without SSE code by saving

Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread Duy Nguyen
On Thu, May 29, 2014 at 11:36 PM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Thu, May 29, 2014 at 6:49 AM, David Turner dtur...@twopensource.com wrote: I assume that most of the time spent in check_refname_component() is while reading the packed-refs

Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread Duy Nguyen
On Fri, May 30, 2014 at 6:41 AM, Jeff King p...@peff.net wrote: On Fri, May 30, 2014 at 06:24:30AM +0700, Duy Nguyen wrote: I wonder if we can get away without SSE code by saving stat info of the packed-refs version that we have verified. When we read pack-refs, if stat info matches, skip

Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread Jeff King
On Fri, May 30, 2014 at 06:43:18AM +0700, Duy Nguyen wrote: The first time we read packed_refs, check_refname_format() is called in read_packed_refs()-create_ref_entry() as usual. If we find no problem, we store packed_refs stat() info in maybe packed_refs.stat. Next time we read

Re: [PATCH] check_refname_component: Optimize

2014-05-29 Thread Duy Nguyen
On Fri, May 30, 2014 at 7:07 AM, Jeff King p...@peff.net wrote: But then we're just trusting that the trust me flag on disk is correct. Why not just trust that packed-refs is correct in the first place? IOW, consider this progression of changes: 1. Check refname format when we read

Re: [PATCH] check_refname_component: Optimize

2014-05-28 Thread Junio C Hamano
David Turner dtur...@twopensource.com writes: In a repository with tens of thousands of refs, the command ~/git/git-diff-index --cached --quiet --ignore-submodules [revision] is a bit slow. check_refname_component is a major contributor to the runtime. Provide two optimized versions of this

Re: [PATCH] check_refname_component: Optimize

2014-05-28 Thread Michael Haggerty
On 05/28/2014 11:04 PM, David Turner wrote: In a repository with tens of thousands of refs, the command ~/git/git-diff-index --cached --quiet --ignore-submodules [revision] is a bit slow. check_refname_component is a major contributor to the runtime. Provide two optimized versions of this

Re: [PATCH] check_refname_component: Optimize

2014-05-28 Thread David Turner
On Wed, 2014-05-28 at 23:44 +0200, Michael Haggerty wrote: On 05/28/2014 11:04 PM, David Turner wrote: In a repository with tens of thousands of refs, the command ~/git/git-diff-index --cached --quiet --ignore-submodules [revision] is a bit slow. check_refname_component is a major