{"thread":{"id":"54288","subject":"[PATCH 0/2] drop unaligned loads","startedAt":"2020-09-24T19:16:41Z","lastAt":"2020-09-25T20:48:17Z","messageCount":9,"participants":["Jeff King","René Scharfe","brian m. carlson","Carlo Arenas","Thomas Guyot"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"406278","messageId":"20200924191638.GA2528003@coredump.intra.peff.net","threadId":"54288","inReplyTo":null,"subject":"[PATCH 0/2] drop unaligned loads","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-24T19:16:38Z","receivedAt":"2020-09-24T19:16:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 24, 2020 at 10:22:20AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Then I did the same, but building with -DNO_UNALIGNED_LOADS. The latter\n> > actually ran faster, by a small margin. Here are the hyperfine results:\n> >\n> >   [stock]\n> >   Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]\n> >   Range (min … max):    6.550 s …  6.841 s    10 runs\n> >\n> >   [-DNO_UNALIGNED_LOADS]\n> >   Time (mean ± σ):      6.418 s ±  0.015 s    [User: 6.058 s, System: 0.360 s]\n> >   Range (min … max):    6.394 s …  6.447 s    10 runs\n> >\n> > For casual use as in reftables I doubt the difference is even\n> > measurable. But this result implies that perhaps we ought to just be\n> > using the fallback version all the time.\n> \n> I like that one.  One less configurable knob that makes us execute\n> different codepaths is one less thing to be worried about.\n\nHere it is with a little more research, then, and a cleanup we can do on\ntop.\n\n  [1/2]: bswap.h: drop unaligned loads\n  [2/2]: Revert \"fast-export: use local array to store anonymized oid\"\n\n Makefile              |  1 -\n builtin/fast-export.c |  8 ++++----\n compat/bswap.h        | 24 ------------------------\n 3 files changed, 4 insertions(+), 29 deletions(-)\n\n-Peff\n"},{"id":"406279","messageId":"20200924192111.GA2528225@coredump.intra.peff.net","threadId":"54288","inReplyTo":"20200924191638.GA2528003@coredump.intra.peff.net","subject":"[PATCH 1/2] bswap.h: drop unaligned loads","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-24T19:21:11Z","receivedAt":"2020-09-24T19:21:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our put_be32() routine and its variants (get_be32(), put_be64(), etc)\nhas two implementations: on some platforms we cast memory in place and\nuse nothl()/htonl(), which can cause unaligned memory access. And on\nothers, we pick out the individual bytes using bitshifts.\n\nThis introduces extra complexity, and sometimes causes compilers to\ngenerate warnings about type-punning. And it's not clear there's any\nperformance advantage.\n\nThis split goes back to 660231aa97 (block-sha1: support for\narchitectures with memory alignment restrictions, 2009-08-12). The\nunaligned versions were part of the original block-sha1 code in\nd7c208a92e (Add new optimized C 'block-sha1' routines, 2009-08-05),\nwhich says it is:\n\n   Based on the mozilla SHA1 routine, but doing the input data accesses a\n   word at a time and with 'htonl()' instead of loading bytes and shifting.\n\nBack then, Linus provided timings versus the mozilla code which showed a\n27% improvement:\n\n  https://lore.kernel.org/git/alpine.LFD.2.01.0908051545000.3390@localhost.localdomain/\n\nHowever, the unaligned loads were either not the useful part of that\nspeedup, or perhaps compilers and processors have changed since then.\nHere are times for computing the sha1 of 4GB of random data, with and\nwithout -DNO_UNALIGNED_LOADS (and BLK_SHA1=1, of course). This is with\ngcc 10, -O2, and the processor is a Core i9-9880H.\n\n  [stock]\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]\n    Range (min … max):    6.550 s …  6.841 s    10 runs\n\n  [-DNO_UNALIGNED_LOADS]\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):      6.418 s ±  0.015 s    [User: 6.058 s, System: 0.360 s]\n    Range (min … max):    6.394 s …  6.447 s    10 runs\n\nAnd here's the same test run on an AMD A8-7600, using gcc 8.\n\n  [stock]\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):     11.721 s ±  0.113 s    [User: 10.761 s, System: 0.951 s]\n    Range (min … max):   11.509 s … 11.861 s    10 runs\n\n  [-DNO_UNALIGNED_LOADS]\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):     11.744 s ±  0.066 s    [User: 10.807 s, System: 0.928 s]\n    Range (min … max):   11.637 s … 11.863 s    10 runs\n\nSo the unaligned loads don't seem to help much, and actually make things\nworse. It's possible there are platforms where they provide more\nbenefit, but:\n\n  - the non-x86 platforms for which we use this code are old and obscure\n    (powerpc and s390).\n\n  - the main caller that cares about performance is block-sha1. But\n    these days it is rarely used anyway, in favor of sha1dc (which is\n    already much slower, and nobody seems to have cared that much).\n\nLet's just drop unaligned versions entirely in the name of simplicity.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n+cc Linus for any thoughts or performance wisdom, and in particular if\n    there might be other platforms worth benchmarking (though unless\n    the improvement is dramatic on some system, avoiding the complexity\n    and pointer aliasing seems worthwhile to me).\n\n+cc René because I know he is going to feed the two of them into\n    godbolt; I could do that, too, but he will provide much better analysis\n    on top ;)\n\n Makefile       |  1 -\n compat/bswap.h | 24 ------------------------\n 2 files changed, 25 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 92d188fd37..d46765aa9e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1218,7 +1218,6 @@ SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))\n BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)\n BASIC_CFLAGS += -fno-omit-frame-pointer\n ifneq ($(filter undefined,$(SANITIZERS)),)\n-BASIC_CFLAGS += -DNO_UNALIGNED_LOADS\n BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS\n endif\n ifneq ($(filter leak,$(SANITIZERS)),)\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex e4e25735ce..c0bb744adc 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -145,28 +145,6 @@ static inline uint64_t git_bswap64(uint64_t x)\n \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- * Otherwise fall back to byte loads and shifts which is portable,\n- * and is faster on architectures with memory alignment issues.\n- */\n-\n-#if !defined(NO_UNALIGNED_LOADS) && ( \\\n-    defined(__i386__) || defined(__x86_64__) || \\\n-    defined(_M_IX86) || defined(_M_X64) || \\\n-    defined(__ppc__) || defined(__ppc64__) || \\\n-    defined(__powerpc__) || defined(__powerpc64__) || \\\n-    defined(__s390__) || defined(__s390x__))\n-\n-#define get_be16(p)\tntohs(*(unsigned short *)(p))\n-#define get_be32(p)\tntohl(*(unsigned int *)(p))\n-#define get_be64(p)\tntohll(*(uint64_t *)(p))\n-#define put_be32(p, v)\tdo { *(unsigned int *)(p) = htonl(v); } while (0)\n-#define put_be64(p, v)\tdo { *(uint64_t *)(p) = htonll(v); } while (0)\n-\n-#else\n-\n static inline uint16_t get_be16(const void *ptr)\n {\n \tconst unsigned char *p = ptr;\n@@ -212,6 +190,4 @@ static inline void put_be64(void *ptr, uint64_t value)\n \tp[7] = value >>  0;\n }\n \n-#endif\n-\n #endif /* COMPAT_BSWAP_H */\n-- \n2.28.0.1082.g195fe9fe3f\n\n"},{"id":"406280","messageId":"20200924192229.GB2528225@coredump.intra.peff.net","threadId":"54288","inReplyTo":"20200924191638.GA2528003@coredump.intra.peff.net","subject":"[PATCH 2/2] Revert \"fast-export: use local array to store anonymized oid\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-24T19:22:29Z","receivedAt":"2020-09-24T19:22:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This reverts commit f39ad38410da554af54966bf74fa0402355852ac.\n\nThat commit was trying to silence a type-punning warning on older\nversions of gcc. However, its analysis was all wrong. I didn't notice\nthat we _were_ in fact type-punning because there are two versions of\nput_be32(): one that uses casts and unaligned loads, and another that\nuses bitshifts. I looked at the latter, but on my platform we were\ndefaulting to the former.\n\nHowever, as of the previous commit, we'll always use the bitshift\nversion. So we can drop this hackery to avoid the warning, making the\ncode slightly cleaner.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/fast-export.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 1b8fca3ee0..e266d90a81 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -405,12 +405,12 @@ static char *generate_fake_oid(void *data)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n-\tunsigned char out[GIT_MAX_RAWSZ];\n+\tstruct object_id oid;\n \tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n \n-\thashclr(out);\n-\tput_be32(out + hashsz - 4, counter++);\n-\treturn hash_to_hex_algop_r(hex, out, the_hash_algo);\n+\toidclr(&oid);\n+\tput_be32(oid.hash + hashsz - 4, counter++);\n+\treturn oid_to_hex_r(hex, &oid);\n }\n \n static const char *anonymize_oid(const char *oid_hex)\n-- \n2.28.0.1082.g195fe9fe3f\n"},{"id":"406288","messageId":"b4b9475f-9c84-1889-835d-9f6e81198e5b@web.de","threadId":"54288","inReplyTo":"20200924192111.GA2528225@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] bswap.h: drop unaligned loads","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-09-24T22:02:38Z","receivedAt":"2020-09-24T22:03:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.09.20 um 21:21 schrieb Jeff King:\n> Our put_be32() routine and its variants (get_be32(), put_be64(), etc)\n> has two implementations: on some platforms we cast memory in place and\n> use nothl()/htonl(), which can cause unaligned memory access. And on\n> others, we pick out the individual bytes using bitshifts.\n>\n> This introduces extra complexity, and sometimes causes compilers to\n> generate warnings about type-punning. And it's not clear there's any\n> performance advantage.\n>\n> This split goes back to 660231aa97 (block-sha1: support for\n> architectures with memory alignment restrictions, 2009-08-12). The\n> unaligned versions were part of the original block-sha1 code in\n> d7c208a92e (Add new optimized C 'block-sha1' routines, 2009-08-05),\n> which says it is:\n>\n>    Based on the mozilla SHA1 routine, but doing the input data accesses a\n>    word at a time and with 'htonl()' instead of loading bytes and shifting.\n>\n> Back then, Linus provided timings versus the mozilla code which showed a\n> 27% improvement:\n>\n>   https://lore.kernel.org/git/alpine.LFD.2.01.0908051545000.3390@localhost.localdomain/\n>\n> However, the unaligned loads were either not the useful part of that\n> speedup, or perhaps compilers and processors have changed since then.\n> Here are times for computing the sha1 of 4GB of random data, with and\n> without -DNO_UNALIGNED_LOADS (and BLK_SHA1=1, of course). This is with\n> gcc 10, -O2, and the processor is a Core i9-9880H.\n>\n>   [stock]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]\n>     Range (min … max):    6.550 s …  6.841 s    10 runs\n>\n>   [-DNO_UNALIGNED_LOADS]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):      6.418 s ±  0.015 s    [User: 6.058 s, System: 0.360 s]\n>     Range (min … max):    6.394 s …  6.447 s    10 runs\n>\n> And here's the same test run on an AMD A8-7600, using gcc 8.\n>\n>   [stock]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):     11.721 s ±  0.113 s    [User: 10.761 s, System: 0.951 s]\n>     Range (min … max):   11.509 s … 11.861 s    10 runs\n>\n>   [-DNO_UNALIGNED_LOADS]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):     11.744 s ±  0.066 s    [User: 10.807 s, System: 0.928 s]\n>     Range (min … max):   11.637 s … 11.863 s    10 runs\n\nYay, benchmarks!  GCC 10.2 with -O2 on an i5-9600K without NO_UNALIGNED_LOADS:\n\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):      6.547 s ±  0.015 s    [User: 6.127 s, System: 0.395 s]\n    Range (min … max):    6.531 s …  6.583 s    10 runs\n\n... and with NO_UNALIGNED_LOADS set:\n\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):      6.496 s ±  0.011 s    [User: 6.135 s, System: 0.360 s]\n    Range (min … max):    6.486 s …  6.519 s    10 runs\n\nclang 10 without NO_UNALIGNED_LOADS:\n\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):      6.697 s ±  0.028 s    [User: 6.343 s, System: 0.354 s]\n    Range (min … max):    6.675 s …  6.754 s    10 runs\n\n... and with NO_UNALIGNED_LOADS set:\n\n  Benchmark #1: t/helper/test-tool sha1 <foo.rand\n    Time (mean ± σ):      6.714 s ±  0.049 s    [User: 6.320 s, System: 0.375 s]\n    Range (min … max):    6.651 s …  6.791 s    10 runs\n\n> +cc René because I know he is going to feed the two of them into\n>     godbolt; I could do that, too, but he will provide much better analysis\n>     on top ;)\n\nWeeell, I don't know about that, but I couldn't resist taking a quick\nlook at what some compilers do with the 32-bit functions, which are the\nones used in block-sha1: https://www.godbolt.org/z/rhKMTM.\n\nOlder versions of gcc and clang didn't see through the shifting\nput_be32() implementation.  If you go back further there are also\nversions that didn't optimize the shifting get_be32().  And the latest\nicc still can't do that.\n\ngcc 10.2 just optimizes all functions to a bswap and a mov.  Can't do\nany better than that, can you?\n\nBut why do we then see a difference in our benchmark results?  Not sure,\nbut https://www.godbolt.org/z/7xh8ao shows that gcc is shuffling some\ninstructions around depending on the implementation.  Switch to clang if\nyou want to see more vigorous shuffling.\n\nThe performance of bigger pieces of code seems to be a matter of luck\nto some extent. :-/\n\nRené\n"},{"id":"406291","messageId":"20200925011348.GA1392312@camp.crustytoothpaste.net","threadId":"54288","inReplyTo":"20200924192111.GA2528225@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] bswap.h: drop unaligned loads","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-09-25T01:13:48Z","receivedAt":"2020-09-25T01:13:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-09-24 at 19:21:11, Jeff King wrote:\n> However, the unaligned loads were either not the useful part of that\n> speedup, or perhaps compilers and processors have changed since then.\n> Here are times for computing the sha1 of 4GB of random data, with and\n> without -DNO_UNALIGNED_LOADS (and BLK_SHA1=1, of course). This is with\n> gcc 10, -O2, and the processor is a Core i9-9880H.\n> \n>   [stock]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]\n>     Range (min … max):    6.550 s …  6.841 s    10 runs\n> \n>   [-DNO_UNALIGNED_LOADS]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):      6.418 s ±  0.015 s    [User: 6.058 s, System: 0.360 s]\n>     Range (min … max):    6.394 s …  6.447 s    10 runs\n> \n> And here's the same test run on an AMD A8-7600, using gcc 8.\n> \n>   [stock]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):     11.721 s ±  0.113 s    [User: 10.761 s, System: 0.951 s]\n>     Range (min … max):   11.509 s … 11.861 s    10 runs\n> \n>   [-DNO_UNALIGNED_LOADS]\n>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>     Time (mean ± σ):     11.744 s ±  0.066 s    [User: 10.807 s, System: 0.928 s]\n>     Range (min … max):   11.637 s … 11.863 s    10 runs\n\nI think this is a fine and desirable change, both for performance and\ncorrectness.  It is, as usual, well explained.\n\n> So the unaligned loads don't seem to help much, and actually make things\n> worse. It's possible there are platforms where they provide more\n> benefit, but:\n> \n>   - the non-x86 platforms for which we use this code are old and obscure\n>     (powerpc and s390).\n\nI cannot speak for s390, since I have never owned one, but my\nunderstanding on unaligned access is that typically there is a tiny\npenalty on x86 (about a cycle) and a more significant penalty on\nPowerPC, although that may have changed with newer POWER chips.  So my\ngut tells me this is an improvement either way, although I no longer own\nany such bootable hardware to measure for certain.\n\nAnyway, as René found, the latest versions of GCC already use the\npeephole optimizer to recognize and optimize this on x86, so I expect\nthey'll do so on other architectures as well.  Byte swapping is a pretty\ncommon operation.\n\n>   - the main caller that cares about performance is block-sha1. But\n>     these days it is rarely used anyway, in favor of sha1dc (which is\n>     already much slower, and nobody seems to have cared that much).\n\nI think block-sha256 uses it as well, but in any case, it's still faster\nthan sha1dc and people who care desperately about performance will use a\ncrypto library instead.\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"406295","messageId":"20200925045620.GA3110076@coredump.intra.peff.net","threadId":"54288","inReplyTo":"b4b9475f-9c84-1889-835d-9f6e81198e5b@web.de","subject":"Re: [PATCH 1/2] bswap.h: drop unaligned loads","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T04:56:20Z","receivedAt":"2020-09-25T04:56:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 25, 2020 at 12:02:38AM +0200, René Scharfe wrote:\n\n> Older versions of gcc and clang didn't see through the shifting\n> put_be32() implementation.  If you go back further there are also\n> versions that didn't optimize the shifting get_be32().  And the latest\n> icc still can't do that.\n> \n> gcc 10.2 just optimizes all functions to a bswap and a mov.  Can't do\n> any better than that, can you?\n> \n> But why do we then see a difference in our benchmark results?  Not sure,\n> but https://www.godbolt.org/z/7xh8ao shows that gcc is shuffling some\n> instructions around depending on the implementation.  Switch to clang if\n> you want to see more vigorous shuffling.\n\nWe do redefine ntohl(), etc in compat/bswap.h. Looking at them, I'd\nthink the result would end up as a bswap instruction, though. And\nindeed, trying to feed that to godbolt results in the same output you\ngot.\n\nIt does sound like older compilers were benefiting from the unaligned\nversions. Building with gcc-4.8 (from debian jessie in a docker\ncontainer on the same machine), I get ~6.25s with the unaligned load\nversus ~6.6s with the bit-shifting code. So that's the opposite of how\nthe newer compiler behaves.\n\nBenchmarking clang-8 (which your results showed doesn't handle the\nshifting version well). It likewise is just _slightly_ slower after my\npatch (11.47s versus 11.57s).\n\nGiven that newer compilers behave the opposite way, and the overall\nsmall magnitude of the impact, I'm still comfortable with the change.\nIt's nice to have a better understanding of how the compiler is\nimpacting it (even if I am still confused how anything at all changes on\nthe newer compilers). Thanks for digging.\n\n-Peff\n"},{"id":"406315","messageId":"CAPUEspjpUeaqCCuBJpQLSTv=C_P4f5h22HoTPm9+9rB7k0NkaQ@mail.gmail.com","threadId":"54288","inReplyTo":"20200925011348.GA1392312@camp.crustytoothpaste.net","subject":"Re: [PATCH 1/2] bswap.h: drop unaligned loads","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2020-09-25T09:05:09Z","receivedAt":"2020-09-25T09:05:23Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Thu, Sep 24, 2020 at 6:15 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On 2020-09-24 at 19:21:11, Jeff King wrote:\n> >   [stock]\n> >   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n> >     Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]\n> >     Range (min … max):    6.550 s …  6.841 s    10 runs\n\nslightly offtopic but what generates this nicely formatted output?\n\n> I cannot speak for s390, since I have never owned one\n\nI happen to be lucky enough to have access to one (RHEL 8.2/z15, gcc\n8.3.1) and seems (third consecutive run):\n\nstock: user: 7.555s, system: 1.191s\n-DNO_UNALIGNED_LOADS: user: 7.561s, system: 1.189s\n\nCarlo\n"},{"id":"406316","messageId":"20200925090906.GA66146@coredump.intra.peff.net","threadId":"54288","inReplyTo":"CAPUEspjpUeaqCCuBJpQLSTv=C_P4f5h22HoTPm9+9rB7k0NkaQ@mail.gmail.com","subject":"Re: [PATCH 1/2] bswap.h: drop unaligned loads","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-25T09:09:06Z","receivedAt":"2020-09-25T09:09:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 25, 2020 at 02:05:09AM -0700, Carlo Arenas wrote:\n\n> > >   [stock]\n> > >   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n> > >     Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]\n> > >     Range (min … max):    6.550 s …  6.841 s    10 runs\n> \n> slightly offtopic but what generates this nicely formatted output?\n\nIt's this:\n\n  https://github.com/sharkdp/hyperfine\n\nIt will actually run both versions and compare them, but it's a little\nmore involved to set up (since you have to do a build step in between).\n\n> > I cannot speak for s390, since I have never owned one\n> \n> I happen to be lucky enough to have access to one (RHEL 8.2/z15, gcc\n> 8.3.1) and seems (third consecutive run):\n> \n> stock: user: 7.555s, system: 1.191s\n> -DNO_UNALIGNED_LOADS: user: 7.561s, system: 1.189s\n\nThanks. That's not too surprising. gcc 8 seems to be able to optimize\nboth versions to the same thing (though I have no idea if s390 has a\nbswap instruction).\n\n-Peff\n"},{"id":"406395","messageId":"6d48c9ee-d8cc-1f22-80bb-f249c94f9767@gmail.com","threadId":"54288","inReplyTo":"CAPUEspjpUeaqCCuBJpQLSTv=C_P4f5h22HoTPm9+9rB7k0NkaQ@mail.gmail.com","subject":"Re: [PATCH 1/2] bswap.h: drop unaligned loads","fromName":"Thomas Guyot","fromEmail":"tguyot@gmail.com","sentAt":"2020-09-25T20:48:11Z","receivedAt":"2020-09-25T20:48:17Z","isPatch":true,"sender":{"key":"tguyot@gmail.com","avatar":"https://avatars.githubusercontent.com/u/403890?v=4"},"body":"On 2020-09-25 05:05, Carlo Arenas wrote:\n> On Thu, Sep 24, 2020 at 6:15 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n>> On 2020-09-24 at 19:21:11, Jeff King wrote:\n>>>   [stock]\n>>>   Benchmark #1: t/helper/test-tool sha1 <foo.rand\n>>>     Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]\n>>>     Range (min … max):    6.550 s …  6.841 s    10 runs\n> \n> slightly offtopic but what generates this nicely formatted output?\n\nIt turns out I may have finally reproduced my diff speed regression at\nhome - just today - and I read this :) I may use it to test, nice timing!\n\nAbout my unrelated regression, TL;DR, have anyone benchmarked under a\nVM? (more context below...)\n\n>> I cannot speak for s390, since I have never owned one\n> \n> I happen to be lucky enough to have access to one (RHEL 8.2/z15, gcc\n> 8.3.1) and seems (third consecutive run):\n> \n> stock: user: 7.555s, system: 1.191s\n> -DNO_UNALIGNED_LOADS: user: 7.561s, system: 1.189s\n\nSo also somewhat offtopic too, but I've been trying lately to reproduce\na speed regression with \"git diff --no-ext-diff --quiet\" (used by git\nprompt in contrib) on some large repos between 2.9.5 and 2.16.2. Overall\nthe diff was over 3x as slow where I tested, and I measured a couple\ntimings between mmap of the same two large files between the two\nversion, the delta was approx 10x! (no other syscalls so it's all\ncomputing and - if my theory proves right - I guess a lot of pagefaults\nor similar handling control back to the host).\n\nI prepped a VM where I will do more testing (I ran one preliminary test\nand there seemed to be a visible difference, but I didn't have a proper\nrepo to test with yet). The point being it might be worth comparing the\ntwo on VM's as well.\n\nRegards,\n\nThomas\n"}]}