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

Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed

From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date
Nov 5, 2007, 20:21 UTC
Message-ID
<472F7B2F.4050608@lsrfire.ath.cx>
In-Reply-To
<7v8x5cqxn0.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
Show 30 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> Unfortunately, we cannot reuse the result of that function, which
>> would be cleaner: there are more users than just git log.  Most
>> notably, git-archive with "$Format:...$" substitution.
> 
> That makes sense.
> 
> 
>> diff --git a/pretty.c b/pretty.c
>> index 490cede..241e91c 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -393,6 +393,7 @@ void format_commit_message(const struct commit *commit,
>>  	int i;
>>  	enum { HEADER, SUBJECT, BODY } state;
>>  	const char *msg = commit->buffer;
>> +	char *active = interp_find_active(format, table, ARRAY_SIZE(table));
>> ...
>> +	if (active[IHASH])
>> +		interp_set_entry(table, IHASH,
>> +				sha1_to_hex(commit->object.sha1));
>> +	if (active[IHASH_ABBREV])
>> +		interp_set_entry(table, IHASH_ABBREV,
>>  			find_unique_abbrev(commit->object.sha1,
>>  				DEFAULT_ABBREV));
> 
> Instead of allocating a separate array and freeing at the end,
> wouldn't it make more sense to have a bitfield that records what
> is used by the format string inside the array elements?

How about (ab)using the value field? Let interp_find_active() mark unneeded entries with NULL, and the rest with some cookie. All table entries with non-NULL values need to be initialized. interp_set_entry() needs to be aware of this cookie, as it mustn't free() it. The cookie could be the address of a static char* in interpolate.c.

Previous: Junio C HamanoNext: Jon Loeliger
Message 7 of 24 in “Make user formatted commit listing less expensive”
  1. 0/3 Make user formatted commit listing less expensiveJohannes Schindelin, Nov 4, 2007
  2. 1/3 Split off the pretty print stuff into its own fileJohannes Schindelin, Nov 4, 2007
  3. Junio C HamanoNov 5, 2007
  4. 2/3 interpolate.[ch]: Add a function to find which interpolations are active.Johannes Schindelin, Nov 4, 2007
  5. 3/3 pretty=format: Avoid some expensive calculations when not neededJohannes Schindelin, Nov 4, 2007
  6. Junio C HamanoNov 5, 2007
  7. René ScharfeNov 5, 2007
  8. Jon LoeligerNov 5, 2007
  9. Johannes SchindelinNov 5, 2007
  10. Junio C HamanoNov 6, 2007
  11. René ScharfeNov 6, 2007
  12. René ScharfeNov 6, 2007
  13. Johannes SchindelinNov 6, 2007
  14. René ScharfeNov 7, 2007
  15. Johannes SchindelinNov 8, 2007
  16. Pierre HabouzitNov 7, 2007
  17. Pierre HabouzitNov 7, 2007
  18. René ScharfeNov 7, 2007
  19. Pierre HabouzitNov 7, 2007
  20. Junio C HamanoNov 7, 2007
  21. René ScharfeNov 9, 2007
  22. Johannes SchindelinNov 6, 2007
  23. 1/2 interpolate.[ch]: Add a function to find which interpolations are active.Johannes Schindelin, Nov 6, 2007
  24. 2/2 pretty=format: Avoid some expensive calculations when not neededJohannes Schindelin, Nov 6, 2007

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.