On Fri, 10 Apr 2009, Linus Torvalds wrote: > > > On Fri, 10 Apr 2009, Linus Torvalds wrote: > > > > Here's a less trivial thing, and slightly more dubious one. > > I'm starting to like it more. > > In particular, pushing the "path_name()" call _into_ the show() function > would seem to allow > > - more clarity into who "owns" the name (ie now when we free the name in > the show_object callback, it's because we generated it ourselves by > calling path_name()) > > - not calling path_name() at all, either because we don't care about the > name in the first place, or because we are actually happy walking the > linked list of "struct name_path *" and the last component. > > Now, I didn't do that latter optimization, because it would require some > more coding, but especially looking at "builtin-pack-objects.c", we really > don't even want the whole pathname, we really would be better off with the > list of path components. > > Why? We use that name for two things: > - add_preferred_base_object(), which actually _wants_ to traverse the > path, and now does it by looking for '/' characters! > - for 'name_hash()', which only cares about the last 16 characters of a > name, so again, generating the full name seems to be just unnecessary > work. > > Anyway, so I didn't look any closer at those things, but it did convince > me that the "show_object()" calling convention was crazy, and we're > actually better off doing _less_ in list-objects.c, and giving people > access to the internal data structures so that they can decide whether > they want to generate a path-name or not. YES! I didn't look at the patch really closely, but this fits pretty well with the philosophy behind pack v4 where path components are stored in a separate table (instead of being duplicated in every tree objects for the same path), hence generating path names on demand would be a real win for those cases where it is not needed. Nicolas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html