Re: [PATCH v3 0/21] pack bitmaps

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Nov 14, 2013, 19:19 UTC
Message-ID: <5285224A.2070606@ramsay1.demon.co.uk>
In-Reply-To: <20131114124157.GA23784@sigill.intra.peff.net>

On 14/11/13 12:41, Jeff King wrote:

Show 6 quoted lines

> Here's another iteration of the pack bitmaps series. Compared to v2, it
> changes:
> 
>  - misc style/typo fixes
> 
>  - portability fixes from Ramsay and Torsten

Unfortunately, I didn't find time this weekend to finish the msvc build fixes. However, after a quick squint at these patches, I think you have almost done it for me! :-D

I must have misunderstood the previous discussion, because my patch was written on the assumption that the ewah directory wouldn't be "git-ified" (e.g. #include git-compat-util.h).

So, most of my patch is no longer necessary, given the use of the git compat header (and removal of system headers). I suspect that you only need to add an '#define PRIx64 "I64x"' definition (Hmm, probably to the compat/mingw.h header).

I won't know for sure until I actually try them out, of course. I will wait until these patches land in pu.

[Note: the msvc build is still broken, but the failure is not caused by these patches. Unfortunately, the tests in t5310-*.sh fail. However, if I include some debug code, the tests pass ... :-P ]

The part of the patch I was still working on was ...

Show 21 quoted lines

> 
>  - count-objects garbage-reporting patch from Duy
> 
>  - disable bitmaps when is_repository_shallow(); this also covers the
>    case where the client is shallow, since we feed pack-objects a
>    --shallow-file in that case. This used to done by checking
>    !internal_rev_list, but that doesn't apply after cdab485.
> 
>  - ewah sources now properly use git-compat-util.h and do not include
>    system headers
> 
>  - the ewah code uses ewah_malloc, ewah_realloc, and so forth to let the
>    project use a particular allocator (and we want to use xmalloc and
>    friends). And we defined those in pack-bitmap.h, but of course that
>    had no effect on the ewah/*.c files that did not include
>    pack-bitmap.h.  Since we are hacking up and git-ifying libewok
>    anyway, we can just set the hardcoded fallback to xmalloc instead of
>    malloc.
> 
>   - the ewah code used gcc's __builtin_ctzll, but did not provide a
>     suitable fallback. We now provide a fallback in C.

... here.

I was messing around with several implementations (including the use of msvc compiler intrinsics) with the intention of doing some timing tests etc. [I suspected my C fallback function (a different implementation to yours) would be slightly faster.]

ATB, Ramsay Jones