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

Re: [PATCH] index-pack: fix allocation of sorted_by_pos array

From
Jeff King <peff@peff.net>
Date
Jul 7, 2015, 16:06 UTC
Message-ID
<20150707160630.GA4456@peff.net>
In-Reply-To
<xmqqvbdw3r40.fsf@gitster.dls.corp.google.com>
On Tue, Jul 07, 2015 at 08:49:19AM -0700, Junio C Hamano wrote:
Show 15 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
> 
> > I keep tripping over this "real_type vs type" in this code. What do
> > you think about renaming "type" field to "in_pack_type" and
> > "real_type" to "canon_type" (or "final_type")? "Real" does not really
> > say anything in this context..
> 
> An unqualified name "type" does bother me for the word to express
> what representation the piece of data uses (i.e. is it a delta, or
> is it a base object of "tree" type, or what).  I think I tried to
> unconfuse myself by saying "representation type" in in-code
> comments, reviews and log messages when it is not clear which kind
> between "in-pack representation" or "Git object type of that stored
> data" a sentence is talking about, and I agree "in_pack_type" would
> be a vast improvement over just "type".

I think this is doubly confusing because pack-objects _does_ use in_pack_type. And its "type" is therefore the "real" object type. Which is the opposite of index-pack, which uses "type" for the in-pack type. So at the very least, we should harmonize these two uses.

Show 9 quoted lines
> Especially, if the other one is renamed with "in_pack_" prefix,
> "real_type" is not just clear enough but is probably better because
> it explains what it is from its "meaning" (i.e. it is the type of
> the Git object, not how it is represented in the pack-stream) than
> "final_type" that is named after "how" it is computed (i.e. it makes
> sense to you only if you know that an in-pack type "this is delta"
> does not have the full information and you have to traverse the
> delta chain and you will finally find out what it is when you hit
> the base representation).

Yeah, I agree real_type is fine when paired with in_pack_type. We might consider modifying pack-objects.h to match (on top of just moving index-pack to in_pack_type).

-Peff
Previous: Junio C HamanoNext: Nguyễn Thái Ngọc Duy
Message 11 of 17 in “nd/slim-index-pack-memory-usage update”
  1. 0/2 nd/slim-index-pack-memory-usage updateNguyễn Thái Ngọc Duy, Apr 18, 2015
  2. 1/2 index-pack: reduce object_entry size to save memoryNguyễn Thái Ngọc Duy, Apr 18, 2015
  3. 2/2 index-pack: kill union delta_base to save memoryNguyễn Thái Ngọc Duy, Apr 18, 2015
  4. Junio C HamanoJul 3, 2015
  5. Duy NguyenJul 3, 2015
  6. index-pack: fix overallocation of sorted_by_pos arrayJunio C Hamano, Jul 4, 2015
  7. index-pack: fix allocation of sorted_by_pos arrayJunio C Hamano, Jul 4, 2015
  8. Junio C HamanoJul 6, 2015
  9. Duy NguyenJul 7, 2015
  10. Junio C HamanoJul 7, 2015
  11. Jeff KingJul 7, 2015
  12. 1/2 index-pack: rename the field "type" to "in_pack_type"Nguyễn Thái Ngọc Duy, Jul 8, 2015
  13. 2/2 pack-objects: rename the field "type" to "real_type"Nguyễn Thái Ngọc Duy, Jul 8, 2015
  14. Jeff KingJul 8, 2015
  15. Duy NguyenJul 8, 2015
  16. Jeff KingJul 8, 2015
  17. Junio C HamanoJul 4, 2015

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.