On Tue, Jul 30, 2019 at 04:02:03PM -0400, Jeff King wrote: > On Tue, Jul 30, 2019 at 03:49:38PM -0400, Todd Zullinger wrote: > > > > Subtest 6 had an ordering issue. We do not know whether > > > the problem is the code or the test result not keeping up > > > with the code changes. > > > > > > --- expect 2019-07-30 16:56:36 +0000 > > > +++ actual 2019-07-30 16:56:36 +0000 > > > @@ -1,6 +1,6 @@ > > > NULL > > > NULL > > > NULL > > > +7c7cd714e262561f73f3079dfca4e8724682ac21 3 > > > 139b20d8e6c5b496de61f033f642d0e3dbff528d 2 > > > d79ce1670bdcb76e6d1da2ae095e890ccb326ae9 1 > > > -7c7cd714e262561f73f3079dfca4e8724682ac21 3 > > > > I hit the same failure while building for Fedora on the > > s390x architecture. I have not dug into it much yet, but > > perhaps this is an endianess issue? > > Ah, of course. Our oid hashing is done by just picking off the first > bytes of the sha1, and it doesn't care about endianness (because these > are just internal-to-memory hashes). Yeah. > We _could_ reconcile that like this: Do we really want that, though? It's a hashmap, after all, and the order of iteration over various hashmap implementations tends to be arbitrary. So an argument could be made that this test is overly specific by expecting a particular order of elements (and perhaps by checking the elements' oid as well), and it would be sufficient to check that it iterates over all elements, no matter the order (IOW sorting 'actual' before the comparison). OTOH, this is not just any hashmap, but an oidmap, and I could imagine that there might be use cases where it would be beneficial if the iteration order were to match the oid order (but don't know whether we actually have such a use case). > I > wonder if it's appreciably less efficient. I'll bet I could nerd-snipe > René into doing a bunch of measurements and explorations of the > disassembled code. ;) Maybe it shows up in an oidmap-specific performance test, but with all that's usually going on in Git hashmap performance tends to be negligible (e.g. it's rarely visible in flame graphs).
