On Wed, Mar 19, 2014 at 9:57 AM, Stefan Zager <sza...@chromium.org> wrote: >> >> I still don't understand how compat/pread.c does not work with pack_fd >> per thread. I don't have Windows to test, but I forced compat/pread.c >> on on Linux with similar pack_fd changes and it worked fine, helgrind >> only complained about progress.c. >> >> A pread() implementation that is thread-safe with condition sounds >> like an invite for trouble later. And I don't think converting read() >> to pread() is a good idea. Platforms that rely on pread() will hit >> first because of more use of compat/pread.c. read() seeks while >> pread() does not, so we have to audit more code.. > > Using one fd per thread is all well and good for something like > index-pack, which only accesses a single pack file. But using that > heuristic to add threading elsewhere is probably not going to work. > For example, I have a patch in progress to add threading to checkout, > and another one planned to add threading to status. In both cases, we > would need one fd per thread per pack file, which is pretty > ridiculous. > > There really aren't very many calls to read() in the code. I don't > think it would be very difficult to eliminate the remaining ones. The > more interesting question, I think is: what platforms still don't have > a thread-safe pread implementation?
I don't want to go too deep down the rabbit hole here. We don't have to solve the read() vs. pread() issue once for all right now; that can wait for another day. The pread() implementation in this patch is certainly no worse than the one in compat/pread.c. Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
