On Wed, Dec 05, 2018 at 07:02:17AM +0100, René Scharfe wrote: > > I actually wrote it that way initially, but doing the hashcpy() in the > > caller is a bit more awkward. My thought was to punt on that until the > > rest of the surrounding code starts handling oids. > > The level of awkwardness is the same to me, but sha1_loose_object_info() > is much longer already, so it might feel worse to add it there. Right, what I meant was that in sha1_loose_object_info(): if (special_case) handle_special_case(); is easier to follow than a block setting up the special case and then calling the function. > This > function is easily converted to struct object_id, though, as its single > caller can pass one on -- this makes the copy unnecessary. If you mean modifying sha1_loose_object_info() to take an oid, then sure, I agree that is a good step forward (and that is exactly the "punt until" moment I meant). > > This patch looks sane. How do you want to handle it? I'd assumed your > > earlier one would be for applying on top, but this one doesn't have a > > commit message. Did you want me to squash down the individual hunks? > > I'm waiting for the first one (object-store: use one oid_array per > subdirectory for loose cache) to be accepted, as it aims to solve a > user-visible performance regression, i.e. that's where the meat is. > I'm particularly interested in performance numbers from Ævar for it. > > I can send the last one properly later, and add patches for converting > quick_has_loose() to struct object_id. Those are just cosmetic.. Great, thanks. I just wanted to make sure these improvements weren't going to slip through the cracks. -Peff
