On Thu, May 30, 2024 at 08:36:25AM -0700, Junio C Hamano wrote: > Hmph, the rationale given by 9fcd9e4e (builtin/mv duplicate string > list memory, 2024-05-27) essentially is "the number of elements are > the same as the number of command line parameters", but I do not > think that is quite correct. > > When you do "mv srcA srcB ... dst", you'd inspect the command line > arguments from left to right, notice that srcA is a directory, find > the cache entries for paths that are inside srcA, append the paths > in that directory to source[] and destination[] array, and extend > argc. "for (i = 0; i < argc; i++)" loop that appends one element to > src_for_dst per iteration ends up running the number of paths being > moved, which can be order of magnitude more than the command line > parameters. > > Of course, if we needed to make copies for correctness reasons (or > to clarify memory ownership semantics), that alone may be a good > justification and we do not need an excuse "it's just a handful of > elements anyway" to begin with. > > Anyway, that is about somebody else's patch, not this one ;-). Heh, good digging. I actually wondered if I was making the same mistake while writing mine, but double-checked that src_dir is not expanded in that way. But I didn't think to check Patrick's original. ;) IMHO it is probably still OK. We are bounded by the number of entries in the index (and we already use proportional memory for other parts of the operation). -Peff
