On Thu, 2018-08-23 at 06:26 -0400, Derrick Stolee wrote:
>
> Around the time that my proposed approaches were getting vetoed for
> alignment issues, I figured I was out of my depth here. I reached out to
> Daniel Lemire (of EWAH bitmap fame) on Twitter [1]. His blog is full of
> posts of
Jeff King writes:
> Actually, what I showed earlier does seem to have some weirdness with
> else-if. But I finally stumbled on something even better:
>
> - oidcmp(a, b) != 0
> + !oideq(a, b)
>
> Because of the isomorphisms that coccinelle knows about, that catches
> everything we want.
Nice
On Fri, Aug 24, 2018 at 12:45:10PM -0400, Derrick Stolee wrote:
> Here are the numbers for Linux:
>
> | | v2.18.0 | v2.19.0-rc0 | HEAD |
> |--|--|-||
> | | 86.5 | 70.739 | 57.266 |
> | | 60.582 | 101.928 | 56.641 |
> | |
On 8/23/2018 4:59 PM, Derrick Stolee wrote:
When I choose my own metrics for performance tests, I like to run at
least 10 runs, remove the largest AND smallest runs from the samples,
and then take the average. I did this manually for 'git rev-list --all
--objects' on git/git and got the
On 8/24/2018 2:45 AM, Jeff King wrote:
On Thu, Aug 23, 2018 at 10:59:55PM -0400, Jeff King wrote:
So I think we have a winner. I'll polish that up into patches and send
it out later tonight.
Oof. This rabbit hole keeps going deeper and deeper. I wrote up my
coccinelle findings separately in:
On Fri, Aug 24 2018, Jeff King wrote:
> On Thu, Aug 23, 2018 at 04:59:27PM -0400, Derrick Stolee wrote:
>
>> Using git/git:
>>
>> Test v2.18.0 v2.19.0-rc0 HEAD
>> -
>> 0001.2: 3.10(3.02+0.08) 3.27(3.17+0.09) +5.5%
On Thu, Aug 23, 2018 at 04:59:27PM -0400, Derrick Stolee wrote:
> Using git/git:
>
> Test v2.18.0 v2.19.0-rc0 HEAD
> -
> 0001.2: 3.10(3.02+0.08) 3.27(3.17+0.09) +5.5% 3.14(3.02+0.11) +1.3%
>
>
On Thu, Aug 23, 2018 at 10:59:55PM -0400, Jeff King wrote:
> So I think we have a winner. I'll polish that up into patches and send
> it out later tonight.
Oof. This rabbit hole keeps going deeper and deeper. I wrote up my
coccinelle findings separately in:
On Thu, Aug 23, 2018 at 07:48:42PM -0700, Jacob Keller wrote:
> Odd...
>
> What about..
>
> - if (oidcmp(a,b))
> + if(!oideq(a,b))
> { ... }
Nope, it doesn't like that syntactically.
> Or maybe you need to use something like
>
> <...
> - if (oidcmp(a,b))
> + if (!oideq(a,b))
> ...>
On Thu, Aug 23, 2018 at 5:16 PM Jeff King wrote:
>
> On Thu, Aug 23, 2018 at 08:06:37PM -0400, Jeff King wrote:
>
> > This almost works:
> >
> > @@
> > expression a, b;
> > statement s;
> > @@
> > - if (oidcmp(a, b)) s
> > + if (!oideq(a, b)) s
> >
> > [...]
>
> > So I really do want
On Thu, Aug 23, 2018 at 08:06:37PM -0400, Jeff King wrote:
> This almost works:
>
> @@
> expression a, b;
> statement s;
> @@
> - if (oidcmp(a, b)) s
> + if (!oideq(a, b)) s
>
> [...]
> So I really do want some way of saying "all of the block, no matter what
> it is". Or of leaving
On Thu, Aug 23, 2018 at 07:40:49PM -0400, Jeff King wrote:
> > You can look for explicitly "if (oidcmp(...))" though. I don't know if
> > you can catch *any* use which degrades to boolean outside of an if
> > statement, but I wouldn't expect there to be too many of those?
>
> Yeah, that was my
On Thu, Aug 23, 2018 at 04:30:10PM -0700, Jacob Keller wrote:
> On Thu, Aug 23, 2018 at 9:30 AM Jeff King wrote:
> > I think that audit isn't actually too bad (but definitely not something
> > we should do for v2.19). The cocci patch I showed earlier hits most of
> > them. It misses the negated
On Thu, Aug 23, 2018 at 9:30 AM Jeff King wrote:
> I think that audit isn't actually too bad (but definitely not something
> we should do for v2.19). The cocci patch I showed earlier hits most of
> them. It misses the negated ones (e.g., "if (oidcmp(...))"). I'm not
> sure if there's a way to ask
On 8/23/2018 2:53 PM, Jeff King wrote:
On Thu, Aug 23, 2018 at 06:26:58AM -0400, Derrick Stolee wrote:
I think you can safely
ignore the rest of it if you are otherwise occupied. Even if v2.19 ships
without some mitigation, I don't know that it's all that big a deal,
given the numbers I
On Thu, Aug 23, 2018 at 06:26:58AM -0400, Derrick Stolee wrote:
> > I think you can safely
> > ignore the rest of it if you are otherwise occupied. Even if v2.19 ships
> > without some mitigation, I don't know that it's all that big a deal,
> > given the numbers I generated (which for some reason
On Thu, Aug 23, 2018 at 06:20:26AM -0700, Junio C Hamano wrote:
> Jeff King writes:
>
> > Here are numbers for p0001.2 run against linux.git on a few
> > versions. This is using -O2 with gcc 8.2.0.
> >
> > Test v2.18.0 v2.19.0-rc0 HEAD
> >
> >
On Thu, Aug 23, 2018 at 06:26:58AM -0400, Derrick Stolee wrote:
> Around the time that my proposed approaches were getting vetoed for
> alignment issues, I figured I was out of my depth here. I reached out to
> Daniel Lemire (of EWAH bitmap fame) on Twitter [1]. His blog is full of
> posts of
Jeff King writes:
> Here are numbers for p0001.2 run against linux.git on a few
> versions. This is using -O2 with gcc 8.2.0.
>
> Test v2.18.0 v2.19.0-rc0 HEAD
>
> --
> 0001.2:
Derrick Stolee writes:
> I was thinking that having a mitigation for 2.19 is best, and then we
> can focus as part of the 2.20 cycle how we can properly avoid this
> cost, especially when 32 is a valid option.
> ...
> ... to
> take a 'hasheq' approach [2] like Peff suggested [3]. Since that
>
On 8/23/2018 1:04 AM, Jeff King wrote:
On Thu, Aug 23, 2018 at 03:47:07AM +, brian m. carlson wrote:
I expect that's going to be the case as well. I have patches that
wire up actual SHA-256 support in my hash-impl branch.
However, having said that, I'm happy to defer to whatever everyone
On Thu, Aug 23, 2018 at 01:02:25AM -0400, Jeff King wrote:
> Here's the patch. For some reason my numbers aren't quite as large as
> they were yesterday (I was very careful to keep the system unloaded
> today, whereas yesterday I was doing a few other things, so perhaps that
> is the difference).
Jeff King wrote:
> Here's the patch. For some reason my numbers aren't quite as large as
> they were yesterday (I was very careful to keep the system unloaded
> today, whereas yesterday I was doing a few other things, so perhaps that
> is the difference).
>
> -- >8 --
> Subject: [PATCH] hashcmp:
On Thu, Aug 23, 2018 at 03:47:07AM +, brian m. carlson wrote:
> I expect that's going to be the case as well. I have patches that
> wire up actual SHA-256 support in my hash-impl branch.
>
> However, having said that, I'm happy to defer to whatever everyone else
> thinks is best for 2.19.
On Wed, Aug 22, 2018 at 07:27:56PM -0700, Jonathan Nieder wrote:
> Jeff King wrote:
>
> > FWIW, it's not 10%. The best I measured was ~4% on a very
> > hashcmp-limited operation, and I suspect even that may be highly
> > dependent on the compiler. We might be able to improve more by
> >
On Wed, Aug 22, 2018 at 10:16:18PM -0400, Jeff King wrote:
> FWIW, it's not 10%. The best I measured was ~4% on a very
> hashcmp-limited operation, and I suspect even that may be highly
> dependent on the compiler. We might be able to improve more by
> sprinkling more asserts around, but there are
Jeff King wrote:
> FWIW, it's not 10%. The best I measured was ~4% on a very
> hashcmp-limited operation, and I suspect even that may be highly
> dependent on the compiler. We might be able to improve more by
> sprinkling more asserts around, but there are 75 mentions of
> the_hash_algo->rawsz. I
On Wed, Aug 22, 2018 at 06:23:43PM -0700, Jonathan Nieder wrote:
> Jeff King wrote:
> >> On Tue, 2018-08-21 at 23:03 -0400, Jeff King wrote:
>
> >>> static inline int hashcmp(const unsigned char *sha1, const unsigned
> >>> char *sha2)
> >>> {
> >>> + assert(the_hash_algo->rawsz == 20);
>
Hi,
Jeff King wrote:
>> On Tue, 2018-08-21 at 23:03 -0400, Jeff King wrote:
>>> static inline int hashcmp(const unsigned char *sha1, const unsigned
>>> char *sha2)
>>> {
>>> + assert(the_hash_algo->rawsz == 20);
>>> return memcmp(sha1, sha2, the_hash_algo->rawsz);
>>> }
[...]
>
On 8/22/2018 12:58 PM, Duy Nguyen wrote:
On Wed, Aug 22, 2018 at 6:49 PM Derrick Stolee wrote:
On 8/22/2018 12:26 PM, Jeff King wrote:
On Wed, Aug 22, 2018 at 06:14:24PM +0200, Duy Nguyen wrote:
On Wed, Aug 22, 2018 at 6:08 PM Duy Nguyen wrote:
On Wed, Aug 22, 2018 at 6:03 PM Jeff King
Jeff King writes:
> On Wed, Aug 22, 2018 at 12:49:34PM -0400, Derrick Stolee wrote:
>
>> > Yes, that was what I meant. We actually did switch to that hand-rolled
>> > loop, but later we went back to memcmp in 0b006014c8 (hashcmp: use
>> > memcmp instead of open-coded loop, 2017-08-09).
>>
>>
On Wed, Aug 22, 2018 at 12:49:34PM -0400, Derrick Stolee wrote:
> > Yes, that was what I meant. We actually did switch to that hand-rolled
> > loop, but later we went back to memcmp in 0b006014c8 (hashcmp: use
> > memcmp instead of open-coded loop, 2017-08-09).
>
> Looking at that commit, I'm
On Wed, Aug 22, 2018 at 6:49 PM Derrick Stolee wrote:
>
> On 8/22/2018 12:26 PM, Jeff King wrote:
> > On Wed, Aug 22, 2018 at 06:14:24PM +0200, Duy Nguyen wrote:
> >
> >> On Wed, Aug 22, 2018 at 6:08 PM Duy Nguyen wrote:
> >>> On Wed, Aug 22, 2018 at 6:03 PM Jeff King wrote:
> On Wed, Aug
On 8/22/2018 12:26 PM, Jeff King wrote:
On Wed, Aug 22, 2018 at 06:14:24PM +0200, Duy Nguyen wrote:
On Wed, Aug 22, 2018 at 6:08 PM Duy Nguyen wrote:
On Wed, Aug 22, 2018 at 6:03 PM Jeff King wrote:
On Wed, Aug 22, 2018 at 07:14:42AM -0400, Derrick Stolee wrote:
The other thing I was
On Wed, Aug 22, 2018 at 06:14:24PM +0200, Duy Nguyen wrote:
> On Wed, Aug 22, 2018 at 6:08 PM Duy Nguyen wrote:
> >
> > On Wed, Aug 22, 2018 at 6:03 PM Jeff King wrote:
> > >
> > > On Wed, Aug 22, 2018 at 07:14:42AM -0400, Derrick Stolee wrote:
> > >
> > > > The other thing I was going to
On Wed, Aug 22, 2018 at 6:08 PM Duy Nguyen wrote:
>
> On Wed, Aug 22, 2018 at 6:03 PM Jeff King wrote:
> >
> > On Wed, Aug 22, 2018 at 07:14:42AM -0400, Derrick Stolee wrote:
> >
> > > The other thing I was going to recommend (and I'll try to test this out
> > > myself later) is to see if
On Wed, Aug 22, 2018 at 6:03 PM Jeff King wrote:
>
> On Wed, Aug 22, 2018 at 07:14:42AM -0400, Derrick Stolee wrote:
>
> > The other thing I was going to recommend (and I'll try to test this out
> > myself later) is to see if 'the_hash_algo->rawsz' is being treated as a
> > volatile variable,
On Wed, Aug 22, 2018 at 10:28:56AM -0400, Derrick Stolee wrote:
> In my testing, I've had the best luck with this change:
>
> diff --git a/cache.h b/cache.h
> index b1fd3d58ab..6c8b51c390 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1023,7 +1023,14 @@ extern const struct object_id null_oid;
>
>
On Wed, Aug 22, 2018 at 08:42:20AM -0400, Paul Smith wrote:
> On Tue, 2018-08-21 at 23:03 -0400, Jeff King wrote:
> > static inline int hashcmp(const unsigned char *sha1, const unsigned
> > char *sha2)
> > {
> > + assert(the_hash_algo->rawsz == 20);
> > return memcmp(sha1, sha2,
On Wed, Aug 22, 2018 at 07:14:42AM -0400, Derrick Stolee wrote:
> The other thing I was going to recommend (and I'll try to test this out
> myself later) is to see if 'the_hash_algo->rawsz' is being treated as a
> volatile variable, since it is being referenced through a pointer. Perhaps
>
On Wed, Aug 22, 2018 at 09:39:57AM +0200, Ævar Arnfjörð Bjarmason wrote:
> > I don't have a good option. The assert() thing works until I add in the
> > "32" branch, but that's just punting the issue off until you add support
> > for the new hash.
> >
> > Hand-rolling our own asm or C is a
On 8/22/2018 1:36 AM, brian m. carlson wrote:
On Tue, Aug 21, 2018 at 11:03:44PM -0400, Jeff King wrote:
So I wonder if there's some other way to tell the compiler that we'll
only have a few values. An enum comes to mind, though I don't think the
enum rules are strict enough to make this
On Tue, 2018-08-21 at 23:03 -0400, Jeff King wrote:
> static inline int hashcmp(const unsigned char *sha1, const unsigned
> char *sha2)
> {
> + assert(the_hash_algo->rawsz == 20);
> return memcmp(sha1, sha2, the_hash_algo->rawsz);
> }
I'm not familiar with Git code, but for most
On 8/22/2018 3:39 AM, Ævar Arnfjörð Bjarmason wrote:
On Wed, Aug 22, 2018 at 8:20 AM Jeff King wrote:
On Wed, Aug 22, 2018 at 05:36:26AM +, brian m. carlson wrote:
On Tue, Aug 21, 2018 at 11:03:44PM -0400, Jeff King wrote:
I don't know if something like this is an improvement or now,
On 8/21/2018 11:36 PM, Jeff King wrote:
On Tue, Aug 21, 2018 at 11:03:44PM -0400, Jeff King wrote:
with the obvious "oideq()" implementation added, that seems to get me to
2-3%. Not _quite_ as good as the original branching version I showed.
And we had to touch all the callsites (although
On Wed, Aug 22, 2018 at 8:20 AM Jeff King wrote:
>
> On Wed, Aug 22, 2018 at 05:36:26AM +, brian m. carlson wrote:
>
> > On Tue, Aug 21, 2018 at 11:03:44PM -0400, Jeff King wrote:
> > > So I wonder if there's some other way to tell the compiler that we'll
> > > only have a few values. An enum
On Wed, Aug 22, 2018 at 05:36:26AM +, brian m. carlson wrote:
> On Tue, Aug 21, 2018 at 11:03:44PM -0400, Jeff King wrote:
> > So I wonder if there's some other way to tell the compiler that we'll
> > only have a few values. An enum comes to mind, though I don't think the
> > enum rules are
On Tue, Aug 21, 2018 at 11:03:44PM -0400, Jeff King wrote:
> So I wonder if there's some other way to tell the compiler that we'll
> only have a few values. An enum comes to mind, though I don't think the
> enum rules are strict enough to make this guarantee (after all, it's OK
> to bitwise-OR
On Tue, Aug 21, 2018 at 11:03:44PM -0400, Jeff King wrote:
> with the obvious "oideq()" implementation added, that seems to get me to
> 2-3%. Not _quite_ as good as the original branching version I showed.
> And we had to touch all the callsites (although arguably that kind of
> "eq" function is
On Wed, Aug 22, 2018 at 12:48:16AM +, brian m. carlson wrote:
> > diff --git a/cache.h b/cache.h
> > index b1fd3d58ab..9c004a26c9 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1023,7 +1023,10 @@ extern const struct object_id null_oid;
> >
> > static inline int hashcmp(const unsigned
On Tue, Aug 21, 2018 at 05:29:24PM -0400, Jeff King wrote:
> 0001.2: rev-list --all --objects 37.07(36.62+0.45) 39.11(38.58+0.51) +5.5%
>
> Less change, but my overall times were smaller, too, so clearly our
> hardware or exact repos are a little bit different. Those numbers seem
> pretty
On Tue, Aug 21, 2018 at 04:41:02PM -0400, Derrick Stolee wrote:
> On 8/20/2018 6:13 PM, Junio C Hamano wrote:
> > An early preview release Git v2.19.0-rc0 is now available for
> > testing at the usual places.
>
> As part of testing the release candidate, I ran the performance suite
> against a
On 8/20/2018 6:13 PM, Junio C Hamano wrote:
An early preview release Git v2.19.0-rc0 is now available for
testing at the usual places.
As part of testing the release candidate, I ran the performance suite
against a fresh clone of the Linux repository using v2.18.0 and
v2.19.0-rc0 (also:
On Mon, Aug 20, 2018 at 5:27 PM Jonathan Nieder wrote:
>
> Jonathan Nieder wrote:
> > Stefan Beller wrote:
> >> Junio C Hamano wrote:
>
> >>> * "git submodule" did not correctly adjust core.worktree setting that
> >>>indicates whether/where a submodule repository has its associated
> >>>
Jonathan Nieder wrote:
> Stefan Beller wrote:
>> Junio C Hamano wrote:
>>> * "git submodule" did not correctly adjust core.worktree setting that
>>>indicates whether/where a submodule repository has its associated
>>>working tree across various state transitions, which has been
>>>
(-cc: other lists)
Stefan Beller wrote:
> Junio C Hamano wrote:
>> * "git submodule" did not correctly adjust core.worktree setting that
>>indicates whether/where a submodule repository has its associated
>>working tree across various state transitions, which has been
>>corrected.
>>
56 matches
Mail list logo
