Subject: Re: [PATCH v3 0/21] pack bitmaps

From: Ramsay Jones <[email-masked]>
Date: Thu, 14 Nov 2013 19:19:38 +0000

On 14/11/13 12:41, Jeff King wrote:
> 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 ...

> 
>  - 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