{"thread":{"id":"35849","subject":"pack bitmap woes on Windows","startedAt":"2014-02-12T07:27:36Z","lastAt":"2014-02-13T08:07:05Z","messageCount":11,"participants":["Johannes Sixt","Erik Faye-Lund","David Kastrup","Duy Nguyen","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"234642","messageId":"52FB2268.7080906@viscovery.net","threadId":"35849","inReplyTo":null,"subject":"pack bitmap woes on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-02-12T07:27:36Z","receivedAt":"2014-02-12T07:27:36Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\nthe following symptoms. I haven't followed the topic. Have there been\npatches floating that addressed the problem in one way or another?\n\n(gdb) run\nStarting program: D:\\Src\\mingw-git\\t\\trash directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap HEAD\n[New thread 3528.0x8d4]\nBitmap v1 test (20 entries loaded)\nFound bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 checksum\n\nBreakpoint 1, die (err=0x5939e9 \"Out of memory, realloc failed\") at usage.c:97\n97 if (die_is_recursing()) {\n(gdb) bt\n#0 die (err=0x5939e9 \"Out of memory, realloc failed\") at usage.c:97\n#1 0x00487c4c in xrealloc (ptr=0x12b10008, size=837107040) at wrapper.c:109\n#2 0x0055572b in ewah_to_bitmap (ewah=0xe58c18) at ewah/bitmap.c:105\n#3 0x005426fc in test_bitmap_walk (revs=0x22f93c) at pack-bitmap.c:954\n#4 0x0046b6ae in cmd_rev_list (argc=2, argv=0x3e263c, prefix=0x0)\n at builtin/rev-list.c:329\n#5 0x00402048 in run_builtin (p=0x56d41c, argc=3, argv=0x3e263c) at git.c:314\n#6 0x0040224f in handle_builtin (argc=3, argv=0x3e263c) at git.c:487\n#7 0x00402351 in run_argv (argcp=0x22ff50, argv=0x22ff38) at git.c:533\n#8 0x0040257f in mingw_main (argc=3, av=0x3e2638) at git.c:616\n#9 0x0040242e in main (argc=4, argv=0x3e2638) at git.c:551\n(gdb) up\n#1 0x00487c4c in xrealloc (ptr=0x12b10008, size=837107040) at wrapper.c:109\n109 die(\"Out of memory, realloc failed\");\n(gdb) up\n#2 0x0055572b in ewah_to_bitmap (ewah=0xe58c18) at ewah/bitmap.c:105\n105 bitmap->words = ewah_realloc(\n(gdb) l\n100 ewah_iterator_init(&it, ewah);\n101\n102 while (ewah_iterator_next(&blowup, &it)) {\n103 if (i >= bitmap->word_alloc) {\n104 bitmap->word_alloc *= 1.5;\n105 bitmap->words = ewah_realloc(\n106 bitmap->words, bitmap->word_alloc * sizeof(eword_t));\n107 }\n108\n109 bitmap->words[i++] = blowup;\n(gdb) info locals\nbitmap = (struct bitmap *) 0xe58aa0\nit = {buffer = 0xe58cd0, buffer_size = 2, pointer = 1, compressed = 52981705,\n literals = 0, rl = 2141159439, lw = 8259520, b = 1}\nblowup = 18446744073709551615\ni = 69758920\n(gdb) info args\newah = (struct ewah_bitmap *) 0xe58c18\n(gdb)\n\nThis is after \"not ok 3 - rev-list --test-bitmap verifies bitmaps\".\nNumerous further test cases fail, but I didn't look at them.\n\n-- Hannes\n"},{"id":"234651","messageId":"CABPQNSZAK2d1hSQNG6uoNHoTJGCiq2UnaBP+_RJkPi47xaxGPQ@mail.gmail.com","threadId":"35849","inReplyTo":"52FB2268.7080906@viscovery.net","subject":"Re: pack bitmap woes on Windows","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-02-12T11:56:48Z","receivedAt":"2014-02-12T11:56:48Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Feb 12, 2014 at 8:27 AM, Johannes Sixt wrote:\n> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n> the following symptoms. I haven't followed the topic. Have there been\n> patches floating that addressed the problem in one way or another?\n>\n> (gdb) run\n> Starting program: D:\\Src\\mingw-git\\t\\trash directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap HEAD\n> [New thread 3528.0x8d4]\n> Bitmap v1 test (20 entries loaded)\n> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 checksum\n>\n> Breakpoint 1, die (err=0x5939e9 \"Out of memory, realloc failed\") at usage.c:97\n> 97 if (die_is_recursing()) {\n> (gdb) bt\n> #0 die (err=0x5939e9 \"Out of memory, realloc failed\") at usage.c:97\n> #1 0x00487c4c in xrealloc (ptr=0x12b10008, size=837107040) at wrapper.c:109\n\n~800 megs is a pretty large allocation for 32-bit systems. What gives?\n"},{"id":"234654","messageId":"52FB6C9A.30204@viscovery.net","threadId":"35849","inReplyTo":"CABPQNSZAK2d1hSQNG6uoNHoTJGCiq2UnaBP+_RJkPi47xaxGPQ@mail.gmail.com","subject":"Re: pack bitmap woes on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-02-12T12:44:10Z","receivedAt":"2014-02-12T12:44:10Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 2/12/2014 12:56, schrieb Erik Faye-Lund:\n> On Wed, Feb 12, 2014 at 8:27 AM, Johannes Sixt wrote:\n>> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n>> the following symptoms. I haven't followed the topic. Have there been\n>> patches floating that addressed the problem in one way or another?\n>>\n>> (gdb) run\n>> Starting program: D:\\Src\\mingw-git\\t\\trash directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap HEAD\n>> [New thread 3528.0x8d4]\n>> Bitmap v1 test (20 entries loaded)\n>> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 checksum\n>>\n>> Breakpoint 1, die (err=0x5939e9 \"Out of memory, realloc failed\") at usage.c:97\n>> 97 if (die_is_recursing()) {\n>> (gdb) bt\n>> #0 die (err=0x5939e9 \"Out of memory, realloc failed\") at usage.c:97\n>> #1 0x00487c4c in xrealloc (ptr=0x12b10008, size=837107040) at wrapper.c:109\n> \n> ~800 megs is a pretty large allocation for 32-bit systems. What gives?\n\nThat's exactly the problem: why would a tiny repository from the test\nsuite require such a large allocation? (Not to mention that the allocation\nultimately fails in my case.)\n"},{"id":"234655","messageId":"87mwhwa3s1.fsf@fencepost.gnu.org","threadId":"35849","inReplyTo":"52FB2268.7080906@viscovery.net","subject":"Re: pack bitmap woes on Windows","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-12T12:55:58Z","receivedAt":"2014-02-12T12:55:58Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Sixt writes:\n\n> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n> the following symptoms. I haven't followed the topic. Have there been\n> patches floating that addressed the problem in one way or another?\n>\n> (gdb) run\n> Starting program: D:\\Src\\mingw-git\\t\\trash directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap HEAD\n> [New thread 3528.0x8d4]\n> Bitmap v1 test (20 entries loaded)\n> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 checksum\n\nDoes reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help?\n\n-- \nDavid Kastrup\n"},{"id":"234656","messageId":"52FB7196.7060209@viscovery.net","threadId":"35849","inReplyTo":"87mwhwa3s1.fsf@fencepost.gnu.org","subject":"Re: pack bitmap woes on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-02-12T13:05:26Z","receivedAt":"2014-02-12T13:05:26Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 2/12/2014 13:55, schrieb David Kastrup:\n> Johannes Sixt writes:\n> \n>> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n>> the following symptoms. I haven't followed the topic. Have there been\n>> patches floating that addressed the problem in one way or another?\n>>\n>> (gdb) run\n>> Starting program: D:\\Src\\mingw-git\\t\\trash directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap HEAD\n>> [New thread 3528.0x8d4]\n>> Bitmap v1 test (20 entries loaded)\n>> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 checksum\n> \n> Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help?\n\nYES! t5310 passes after reverting this commit.\n\n-- Hannes\n"},{"id":"234657","messageId":"87ioska2iu.fsf@fencepost.gnu.org","threadId":"35849","inReplyTo":"52FB7196.7060209@viscovery.net","subject":"Re: pack bitmap woes on Windows","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-12T13:23:05Z","receivedAt":"2014-02-12T13:23:05Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Sixt writes:\n\n> Am 2/12/2014 13:55, schrieb David Kastrup:\n>> Johannes Sixt writes:\n>> \n>>> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n>>> the following symptoms. I haven't followed the topic. Have there been\n>>> patches floating that addressed the problem in one way or another?\n>>>\n>>> (gdb) run\n>>> Starting program: D:\\Src\\mingw-git\\t\\trash\n>>> directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap\n>>> HEAD\n>>> [New thread 3528.0x8d4]\n>>> Bitmap v1 test (20 entries loaded)\n>>> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits\n>>> / 15873b36 checksum\n>> \n>> Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help?\n>\n> YES! t5310 passes after reverting this commit.\n\nOh. I just looked through the backtrace until finding a routine\nreasonably related with the regtest and checked for the last commit\nchanging it, then posted my question.\n\nThen I looked through the diff of the patch and considered it\nunconspicuous. So I commenced reading through earlier commits.\n\nI actually don't have a good idea what might be wrong here. The code is\nsomewhat distasteful as it basically uses eword_t and uint64_t\ninterchangeably, but then this does match its current definition.\n\n-- \nDavid Kastrup\n"},{"id":"234658","messageId":"CACsJy8CW0shw5JYtXScQHMYJ+D18-UJ6HanP-5OCuv87kHbnBg@mail.gmail.com","threadId":"35849","inReplyTo":"87ioska2iu.fsf@fencepost.gnu.org","subject":"Re: pack bitmap woes on Windows","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-12T14:09:41Z","receivedAt":"2014-02-12T14:09:41Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 12, 2014 at 8:23 PM, David Kastrup wrote:\n> Johannes Sixt writes:\n>\n>> Am 2/12/2014 13:55, schrieb David Kastrup:\n>>> Johannes Sixt writes:\n>>>\n>>>> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n>>>> the following symptoms. I haven't followed the topic. Have there been\n>>>> patches floating that addressed the problem in one way or another?\n>>>>\n>>>> (gdb) run\n>>>> Starting program: D:\\Src\\mingw-git\\t\\trash\n>>>> directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap\n>>>> HEAD\n>>>> [New thread 3528.0x8d4]\n>>>> Bitmap v1 test (20 entries loaded)\n>>>> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits\n>>>> / 15873b36 checksum\n>>>\n>>> Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help?\n>>\n>> YES! t5310 passes after reverting this commit.\n>\n> Oh. I just looked through the backtrace until finding a routine\n> reasonably related with the regtest and checked for the last commit\n> changing it, then posted my question.\n>\n> Then I looked through the diff of the patch and considered it\n> unconspicuous. So I commenced reading through earlier commits.\n>\n> I actually don't have a good idea what might be wrong here. The code is\n> somewhat distasteful as it basically uses eword_t and uint64_t\n> interchangeably, but then this does match its current definition.\n\nPerhaps __BYTE_ORDER or __BIG_ENDIAN is misdefined and the ntohll() is skipped?\n-- \nDuy\n"},{"id":"234659","messageId":"CABPQNSabAcHEO0+ht7te9ScVigGOoCLdtiD10ye8AqqeLMagLw@mail.gmail.com","threadId":"35849","inReplyTo":"CACsJy8CW0shw5JYtXScQHMYJ+D18-UJ6HanP-5OCuv87kHbnBg@mail.gmail.com","subject":"Re: pack bitmap woes on Windows","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-02-12T14:22:50Z","receivedAt":"2014-02-12T14:22:50Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Feb 12, 2014 at 3:09 PM, Duy Nguyen wrote:\n> On Wed, Feb 12, 2014 at 8:23 PM, David Kastrup wrote:\n>> Johannes Sixt writes:\n>>\n>>> Am 2/12/2014 13:55, schrieb David Kastrup:\n>>>> Johannes Sixt writes:\n>>>>\n>>>>> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n>>>>> the following symptoms. I haven't followed the topic. Have there been\n>>>>> patches floating that addressed the problem in one way or another?\n>>>>>\n>>>>> (gdb) run\n>>>>> Starting program: D:\\Src\\mingw-git\\t\\trash\n>>>>> directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap\n>>>>> HEAD\n>>>>> [New thread 3528.0x8d4]\n>>>>> Bitmap v1 test (20 entries loaded)\n>>>>> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits\n>>>>> / 15873b36 checksum\n>>>>\n>>>> Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help?\n>>>\n>>> YES! t5310 passes after reverting this commit.\n>>\n>> Oh. I just looked through the backtrace until finding a routine\n>> reasonably related with the regtest and checked for the last commit\n>> changing it, then posted my question.\n>>\n>> Then I looked through the diff of the patch and considered it\n>> unconspicuous. So I commenced reading through earlier commits.\n>>\n>> I actually don't have a good idea what might be wrong here. The code is\n>> somewhat distasteful as it basically uses eword_t and uint64_t\n>> interchangeably, but then this does match its current definition.\n>\n> Perhaps __BYTE_ORDER or __BIG_ENDIAN is misdefined and the ntohll() is skipped?\n\nThat is indeed the case.\n"},{"id":"234661","messageId":"CABPQNSZRfnp8aZuKBc=y=iDVdMUo8mKPsLsqg0E+bK1TdNvfkA@mail.gmail.com","threadId":"35849","inReplyTo":"CABPQNSabAcHEO0+ht7te9ScVigGOoCLdtiD10ye8AqqeLMagLw@mail.gmail.com","subject":"Re: pack bitmap woes on Windows","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-02-12T14:49:24Z","receivedAt":"2014-02-12T14:49:24Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Feb 12, 2014 at 3:22 PM, Erik Faye-Lund wrote:\n> On Wed, Feb 12, 2014 at 3:09 PM, Duy Nguyen wrote:\n>> On Wed, Feb 12, 2014 at 8:23 PM, David Kastrup wrote:\n>>> Johannes Sixt writes:\n>>>\n>>>> Am 2/12/2014 13:55, schrieb David Kastrup:\n>>>>> Johannes Sixt writes:\n>>>>>\n>>>>>> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with\n>>>>>> the following symptoms. I haven't followed the topic. Have there been\n>>>>>> patches floating that addressed the problem in one way or another?\n>>>>>>\n>>>>>> (gdb) run\n>>>>>> Starting program: D:\\Src\\mingw-git\\t\\trash\n>>>>>> directory.t5310-pack-bitmaps/..\\..\\git.exe rev-list --test-bitmap\n>>>>>> HEAD\n>>>>>> [New thread 3528.0x8d4]\n>>>>>> Bitmap v1 test (20 entries loaded)\n>>>>>> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits\n>>>>>> / 15873b36 checksum\n>>>>>\n>>>>> Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help?\n>>>>\n>>>> YES! t5310 passes after reverting this commit.\n>>>\n>>> Oh. I just looked through the backtrace until finding a routine\n>>> reasonably related with the regtest and checked for the last commit\n>>> changing it, then posted my question.\n>>>\n>>> Then I looked through the diff of the patch and considered it\n>>> unconspicuous. So I commenced reading through earlier commits.\n>>>\n>>> I actually don't have a good idea what might be wrong here. The code is\n>>> somewhat distasteful as it basically uses eword_t and uint64_t\n>>> interchangeably, but then this does match its current definition.\n>>\n>> Perhaps __BYTE_ORDER or __BIG_ENDIAN is misdefined and the ntohll() is skipped?\n>\n> That is indeed the case.\n\nLooking a bit at it, the whole byte-order detection mess seems insane to me.\n\nMinGW falls inside the \"defined(__GNUC__) && (defined(__i386__) ||\ndefined(__x86_64__))\"-block, but does not define __BYTE_ORDER.\n\nIt seems the author of a201c20b41a2f0725977bcb89a2a66135d776ba2\nassumes __BYTE_ORDER was guaranteed to always be defined, probably\nfooled by the following check:\n\n#if !defined(__BYTE_ORDER)\n# error \"Cannot determine endianness\"\n#endif\n\nHowever, that check is only performed if we're NOT building with GCC\non x86/x64 nor MSVC (which I don't think defined __BYTE_ORDER either)\n\nThe following would make __BYTE_ORDER a guarantee, and show that MinGW\ndoes not define it:\n\n---8<---\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex 120c6c1..c73bf0e 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -109,10 +109,6 @@ static inline uint64_t git_bswap64(uint64_t x)\n # endif\n #endif\n\n-#if !defined(__BYTE_ORDER)\n-# error \"Cannot determine endianness\"\n-#endif\n-\n #if __BYTE_ORDER == __BIG_ENDIAN\n # define ntohll(n) (n)\n # define htonll(n) (n)\n@@ -123,6 +119,10 @@ static inline uint64_t git_bswap64(uint64_t x)\n\n #endif\n\n+#if !defined(__BYTE_ORDER)\n+# error \"Cannot determine endianness\"\n+#endif\n+\n /*\n * Performance might be improved if the CPU architecture is OK with\n * unaligned 32-bit loads and a fast ntohl() is available.\n\n---8<---\n\nBut another level of insanity (IMO) is defining double-underscore\nmacros. These symbols are reserved for the implementation. Sure,\nknowing we're on a given implementation and defining something *else*\ndependent on them is fine. But defining them is just yuckiness, and\nnot very standard-kosher.\n\nIMO, we should rather do the definition the other way around:\n\n#if !defined(BYTE_ORDER)\n# if defined(__BYTE_ORDER) && defined(__LITTLE_ENDIAN) && defined(__BIG_ENDIAN)\n# define BYTE_ORDER __BYTE_ORDER\n# define LITTLE_ENDIAN __LITTLE_ENDIAN\n# define BIG_ENDIAN __BIG_ENDIAN\n# endif\n#endif\n\n...and only referred to BYTE_ORDER, LITTLE_ENDIAN and BIG_ENDIAN.\n\n\nThat way we'd follow closer to where opengroup are heading:\n\nhttp://www.opengroup.org/austin/docs/austin_514.txt\n"},{"id":"234664","messageId":"20140212164828.GA24484@sigill.intra.peff.net","threadId":"35849","inReplyTo":"CABPQNSZRfnp8aZuKBc=y=iDVdMUo8mKPsLsqg0E+bK1TdNvfkA@mail.gmail.com","subject":"Re: pack bitmap woes on Windows","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-12T16:48:28Z","receivedAt":"2014-02-12T16:48:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 12, 2014 at 03:49:24PM +0100, Erik Faye-Lund wrote:\n\n> It seems the author of a201c20b41a2f0725977bcb89a2a66135d776ba2\n> assumes __BYTE_ORDER was guaranteed to always be defined, probably\n> fooled by the following check:\n> \n> #if !defined(__BYTE_ORDER)\n> # error \"Cannot determine endianness\"\n> #endif\n> \n> However, that check is only performed if we're NOT building with GCC\n> on x86/x64 nor MSVC (which I don't think defined __BYTE_ORDER either)\n> \n> The following would make __BYTE_ORDER a guarantee, and show that MinGW\n> does not define it:\n\nYes, that is the problem. Sorry, this got brought up earlier[1], but the\ndiscussion went on and I did not notice that Brian's patch did not get\napplied.\n\nAfter re-reading the discussion, I think the patch below is the best\nsolution. Can you confirm that it fixes the problem for you?\n\n-Peff\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/241278\n\n-- >8 --\nSubject: ewah: unconditionally ntohll ewah data\n\nCommit a201c20 tried to optimize out a loop like:\n\n for (i = 0; i < len; i++)\n\t data[i] = ntohll(data[i]);\n\nin the big-endian case, because we know that ntohll is a\nnoop, and we do not need to pay the cost of the loop at all.\nHowever, it mistakenly assumed that __BYTE_ORDER was always\ndefined, whereas it may not be on systems which do not\ndefine it by default, and where we did not need to define it\nto set up the ntohll macro. This includes OS X and Windows.\n\nWe could muck with the ordering in compat/bswap.h to make\nsure it is defined unconditionally, but it is simpler to\nstill to just execute the loop unconditionally. That avoids\nthe application code knowing anything about these magic\nmacros, and lets it depend only on having ntohll defined.\n\nAnd since the resulting loop looks like (on a big-endian\nsystem):\n\n for (i = 0; i < len; i++)\n\t data[i] = data[i];\n\nany decent compiler can probably optimize it out.\n\nOriginal report and analysis by Brian Gernhardt.\n\nSigned-off-by: Jeff King \n---\n ewah/ewah_io.c | 10 +++-------\n 1 file changed, 3 insertions(+), 7 deletions(-)\n\ndiff --git a/ewah/ewah_io.c b/ewah/ewah_io.c\nindex 4a7fae6..f7f700e 100644\n--- a/ewah/ewah_io.c\n+++ b/ewah/ewah_io.c\n@@ -113,6 +113,7 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)\n int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)\n {\n \tuint8_t *ptr = map;\n+\tsize_t i;\n \n \tself->bit_size = get_be32(ptr);\n \tptr += sizeof(uint32_t);\n@@ -135,13 +136,8 @@ int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)\n \tmemcpy(self->buffer, ptr, self->buffer_size * sizeof(uint64_t));\n \tptr += self->buffer_size * sizeof(uint64_t);\n \n-#if __BYTE_ORDER != __BIG_ENDIAN\n-\t{\n-\t\tsize_t i;\n-\t\tfor (i = 0; i < self->buffer_size; ++i)\n-\t\t\tself->buffer[i] = ntohll(self->buffer[i]);\n-\t}\n-#endif\n+\tfor (i = 0; i < self->buffer_size; ++i)\n+\t\tself->buffer[i] = ntohll(self->buffer[i]);\n \n \tself->rlw = self->buffer + get_be32(ptr);\n \n-- \n1.8.5.2.500.g8060133\n"},{"id":"234709","messageId":"52FC7D29.5070900@viscovery.net","threadId":"35849","inReplyTo":"20140212164828.GA24484@sigill.intra.peff.net","subject":"Re: pack bitmap woes on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-02-13T08:07:05Z","receivedAt":"2014-02-13T08:07:05Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 2/12/2014 17:48, schrieb Jeff King:\n> On Wed, Feb 12, 2014 at 03:49:24PM +0100, Erik Faye-Lund wrote:\n> \n>> It seems the author of a201c20b41a2f0725977bcb89a2a66135d776ba2\n>> assumes __BYTE_ORDER was guaranteed to always be defined, probably\n>> fooled by the following check:\n>>\n>> #if !defined(__BYTE_ORDER)\n>> # error \"Cannot determine endianness\"\n>> #endif\n>>\n>> However, that check is only performed if we're NOT building with GCC\n>> on x86/x64 nor MSVC (which I don't think defined __BYTE_ORDER either)\n>>\n>> The following would make __BYTE_ORDER a guarantee, and show that MinGW\n>> does not define it:\n> \n> Yes, that is the problem. Sorry, this got brought up earlier[1], but the\n> discussion went on and I did not notice that Brian's patch did not get\n> applied.\n> \n> After re-reading the discussion, I think the patch below is the best\n> solution. Can you confirm that it fixes the problem for you?\n\nYes, it fixes the problem! t5310-pack-bitmaps passes all tests with the patch.\n\nThanks,\n-- Hannes\n\n> \n> -Peff\n> \n> [1] http://thread.gmane.org/gmane.comp.version-control.git/241278\n> \n> -- >8 --\n> Subject: ewah: unconditionally ntohll ewah data\n> \n> Commit a201c20 tried to optimize out a loop like:\n> \n> for (i = 0; i < len; i++)\n> \t data[i] = ntohll(data[i]);\n> \n> in the big-endian case, because we know that ntohll is a\n> noop, and we do not need to pay the cost of the loop at all.\n> However, it mistakenly assumed that __BYTE_ORDER was always\n> defined, whereas it may not be on systems which do not\n> define it by default, and where we did not need to define it\n> to set up the ntohll macro. This includes OS X and Windows.\n> \n> We could muck with the ordering in compat/bswap.h to make\n> sure it is defined unconditionally, but it is simpler to\n> still to just execute the loop unconditionally. That avoids\n> the application code knowing anything about these magic\n> macros, and lets it depend only on having ntohll defined.\n> \n> And since the resulting loop looks like (on a big-endian\n> system):\n> \n> for (i = 0; i < len; i++)\n> \t data[i] = data[i];\n> \n> any decent compiler can probably optimize it out.\n> \n> Original report and analysis by Brian Gernhardt.\n> \n> Signed-off-by: Jeff King \n> ---\n> ewah/ewah_io.c | 10 +++-------\n> 1 file changed, 3 insertions(+), 7 deletions(-)\n> \n> diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c\n> index 4a7fae6..f7f700e 100644\n> --- a/ewah/ewah_io.c\n> +++ b/ewah/ewah_io.c\n> @@ -113,6 +113,7 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)\n> int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)\n> {\n> \tuint8_t *ptr = map;\n> +\tsize_t i;\n> \n> \tself->bit_size = get_be32(ptr);\n> \tptr += sizeof(uint32_t);\n> @@ -135,13 +136,8 @@ int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)\n> \tmemcpy(self->buffer, ptr, self->buffer_size * sizeof(uint64_t));\n> \tptr += self->buffer_size * sizeof(uint64_t);\n> \n> -#if __BYTE_ORDER != __BIG_ENDIAN\n> -\t{\n> -\t\tsize_t i;\n> -\t\tfor (i = 0; i < self->buffer_size; ++i)\n> -\t\t\tself->buffer[i] = ntohll(self->buffer[i]);\n> -\t}\n> -#endif\n> +\tfor (i = 0; i < self->buffer_size; ++i)\n> +\t\tself->buffer[i] = ntohll(self->buffer[i]);\n> \n> \tself->rlw = self->buffer + get_be32(ptr);\n> \n> \n"}]}