git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] process_{tree,blob}: Remove useless xstrdup calls

From
Nicolas Pitre <nico@cam.org>
Date
Apr 11, 2009, 01:34 UTC
Message-ID
<alpine.LFD.2.00.0904102129510.6741@xanadu.home>
In-Reply-To
<alpine.LFD.2.00.0904101806340.4583@localhost.localdomain>
On Fri, 10 Apr 2009, Linus Torvalds wrote:
Show 36 quoted lines
> 
> 
> 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
Previous: Linus TorvaldsNext: Björn Steinbrink
Message 59 of 97 in “Performance issue: initial git clone causes massive repack”
  1. Robin H. JohnsonApr 4, 2009
  2. Nicolas SebrechtApr 5, 2009
  3. Robin H. JohnsonApr 5, 2009
  4. Nicolas SebrechtApr 5, 2009
  5. Nicolas SebrechtApr 5, 2009
  6. Robin H. JohnsonApr 5, 2009
  7. Nicolas SebrechtApr 5, 2009
  8. Shawn O. PearceApr 5, 2009
  9. Robin H. JohnsonApr 5, 2009
  10. Robin H. JohnsonApr 5, 2009
  11. Shawn O. PearceApr 5, 2009
  12. david@lang.hmApr 5, 2009
  13. Sverre RabbelierApr 5, 2009
  14. Nicolas PitreApr 6, 2009
  15. Björn SteinbrinkApr 7, 2009
  16. Jakub NarebskiApr 7, 2009
  17. Nicolas PitreApr 7, 2009
  18. Jakub NarebskiApr 7, 2009
  19. Jon SmirlApr 7, 2009
  20. Nicolas PitreApr 7, 2009
  21. Björn SteinbrinkApr 7, 2009
  22. Nicolas PitreApr 7, 2009
  23. Björn SteinbrinkApr 7, 2009
  24. Nicolas PitreApr 7, 2009
  25. Björn SteinbrinkApr 7, 2009
  26. Nicolas PitreApr 8, 2009
  27. Robin H. JohnsonApr 10, 2009
  28. Nicolas PitreApr 11, 2009
  29. Mike HommeyApr 11, 2009
  30. Johannes SchindelinApr 14, 2009
  31. Nicolas PitreApr 14, 2009
  32. Robin H. JohnsonApr 14, 2009
  33. Nicolas PitreApr 14, 2009
  34. Nguyen Thai Ngoc DuyApr 15, 2009
  35. Robin H. JohnsonApr 15, 2009
  36. Junio C HamanoApr 15, 2009
  37. Nicolas PitreApr 15, 2009
  38. Sam VilainApr 22, 2009
  39. Mike RalphsonApr 22, 2009
  40. Pieter de BieApr 22, 2009
  41. Johannes SchindelinApr 22, 2009
  42. Shawn O. PearceApr 22, 2009
  43. Andreas EricssonApr 22, 2009
  44. Johannes SchindelinApr 22, 2009
  45. Christian CouderApr 23, 2009
  46. Nicolas PitreApr 22, 2009
  47. Sam VilainApr 22, 2009
  48. Björn SteinbrinkApr 22, 2009
  49. Nicolas PitreApr 22, 2009
  50. Johannes SchindelinApr 22, 2009
  51. Nicolas PitreApr 23, 2009
  52. Johannes SchindelinApr 14, 2009
  53. Jeff KingApr 7, 2009
  54. Björn SteinbrinkApr 7, 2009
  55. process_{tree,blob}: Remove useless xstrdup callsBjörn Steinbrink, Apr 8, 2009
  56. Linus TorvaldsApr 10, 2009
  57. Linus TorvaldsApr 11, 2009
  58. Linus TorvaldsApr 11, 2009
  59. Nicolas PitreApr 11, 2009
  60. Björn SteinbrinkApr 11, 2009
  61. Björn SteinbrinkApr 11, 2009
  62. Linus TorvaldsApr 11, 2009
  63. Linus TorvaldsApr 11, 2009
  64. Björn SteinbrinkApr 11, 2009
  65. Björn SteinbrinkApr 11, 2009
  66. Linus TorvaldsApr 11, 2009
  67. Björn SteinbrinkApr 11, 2009
  68. Linus TorvaldsApr 11, 2009
  69. Björn SteinbrinkApr 11, 2009
  70. Linus TorvaldsApr 11, 2009
  71. Nicolas SebrechtApr 5, 2009
  72. david@lang.hmApr 5, 2009
  73. Robin RosenbergApr 5, 2009
  74. Nicolas PitreApr 6, 2009
  75. Junio C HamanoApr 6, 2009
  76. Nicolas PitreApr 6, 2009
  77. Jon SmirlApr 6, 2009
  78. Nicolas PitreApr 6, 2009
  79. Jon SmirlApr 6, 2009
  80. Shawn O. PearceApr 6, 2009
  81. Nicolas PitreApr 6, 2009
  82. Jon SmirlApr 6, 2009
  83. Nicolas PitreApr 6, 2009
  84. Matthieu MoyApr 6, 2009
  85. Nicolas PitreApr 6, 2009
  86. Robin H. JohnsonApr 6, 2009
  87. Nicolas PitreApr 6, 2009
  88. Martin LanghoffApr 7, 2009
  89. Jeff KingApr 5, 2009
  90. Robin H. JohnsonApr 5, 2009
  91. Robin H. JohnsonApr 5, 2009
  92. Nguyen Thai Ngoc DuyApr 6, 2009
  93. Nicolas PitreApr 6, 2009
  94. Nicolas PitreApr 6, 2009
  95. Robin H. JohnsonApr 6, 2009
  96. Mark LevedahlApr 11, 2009
  97. Robin H. JohnsonApr 6, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.