Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: >> This comment makes me wonder if it would be even better to >> >> - prepare an empty to_fetch OID array in the caller, >> >> - if the output format is one of the ones that wants prefetch, add >> object names to to_fetch in the caller, BUT not fetch there. >> >> - pass &to_fetch by the caller to this function, and this code here >> may add even more objects, >> >> - then do the prefetch here (so a single promisor interaction will >> grab objects the caller would have fetched before calling us and >> the ones we want here), and then clear the to_fetch array. >> >> - the caller, after seeing this function returns, checks to_fetch >> and if it is not empty, fetches (i.e. the caller prepared list of >> objects based on the output type, we ended up not calling this >> helper, and then finally the caller does the prefetch). >> >> That way, the "unless we have already prefetched" logic can go, and >> we can lose one indentation level, no? > > This means that the only prefetch occurs in diffcore_rename()? No, but I phrased the last bullet item incorrectly. "after seeing this function returns" is wrong, but what is in parentheses (i.e. if we didn't call diffcore_rename) is correct. > I don't > think this will work for 2 reasons: > > - diffcore_std() calls diffcore_break() (which also reads blobs) before > diffcore_rename() Ahh, I missed that part. > - (more importantly) there's a code path in diffcore_std() that does > not call diffcore_rename(), so we would still need some prefetching > logic in diffcore_std() in case diffcore_rename() is not called That one I think is already covered.
