On Fri, Nov 04, 2022 at 11:17:10AM +0800, Teng Long wrote: > From: Teng Long <dyroneteng@xxxxxxxxx> > > Everytime when calling "open_pack_bitmap_1()", we will firstly call > "open_pack_index(packfile)" to check the index, then further check > again in "is_pack_valid()" before mmap the bitmap file. So, let's > remove the first check here. > > The relate discussion: > https://public-inbox.org/git/Y2IiSU1L+bJPUioV@xxxxxxxxxxxxxxxxxxxxxxx/#t The related discussion is good, but I would have liked to see some of the details make its way into the patch message for future readers. Maybe something like: When trying to open a pack bitmap, we call open_pack_bitmap_1() in a loop, during which it tries to open up the pack index corresponding with each available pack. It's likely that we'll end up relying on objects in that pack later in the process (in which case we're doing the work of opening the pack index optimistically), but not guaranteed. For instance, consider a repository with a large number of small packs, and one large pack with a bitmap. If we see that bitmap pack last in our loop which calls open_pack_bitmap_1(), the current code will have opened *all* pack index files in the repository. If the request can be served out of the bitmapped pack alone, then the time spent opening these idx files was wasted.S Since open_pack_bitmap_1() calls is_pack_valid() later on (which in turns calls open_pack_index() itself), we can just drop the earlier call altogether. Thanks, Taylor
