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

Re: [PATCH/RFC v2 0/16] Introduce index file format version 5

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 8, 2012, 01:38 UTC
Message-ID
<7v7gtaups3.fsf@alter.siamese.dyndns.org>
In-Reply-To
<CACsJy8CepAQr4r-c_hzwgT2-_bpFV-=VmOsFaBcDZqBpG52reQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> add_to_index and remove_index_entry_at seem good places for the cut.
> But do we need to redefine the location?

That is one of the things we need to think about carefully. Of course, if add_to_index() just takes a pathname out of the ce the caller wants to add, you can define remove_from_index() that takes a pathname (and possibly a stage), finds the ce with that pathname in the index and removes it. But that would unnecessrily penalize the callers that follow "see if there is such an entry (i.e. "locate"), optionally inspect the entry and then decide to remove", especially if the "locate" is expensive. If a nonsignificant number of callers follow such a pattern, then having a separate "locate" API that can return a "location" that is expressed either as the number of entries to skip from the beginning (in a flat in-core array) or a pair of in-core "directory" structure and the index in the directory to let the caller find the entry quickly, and then later pass it to "remove", would be more appropriate. add_index_entry_at() may also not a bad thing to have if many callers turn out to follow a similar access pattern (i.e. locate, decide to or not to replace when there already is one, and then add it).

Show 6 quoted lines
>  - for 3-5 years since v5 is released, we support v2 and v5 in
> parallel. Other code can take advantage of v5, but it must neither
> sacrifice v2 performance, compatibility nor maintainability
>  - after that, we deprecate v2. v2 is automatically converted to v5 in
> memory. v2 perf may suffer but at that point we don't care any more as
> the majority of users should have been migrated to v5 (*)

As long as the performance of Git on a working tree that used to get certain performance back when it was using v2 does not degrade when it is converted to v5 or later, I think the above is a good way forward.

> If the long term plan is actually that, we will need to stick to flat
> array implementation for forseeable future as moving from it most
> likely impacts v2 performance.

I do not see why we need to "stick to"; I do not see why it is necessarily a bad thing if we end up choosing to "stick to" if the reason we choose it is because the flat in-core performs better.

If the workload we _care_ about is served better by using an API that works over an in-core tree-shaped index data structure, I do not think it is unreasonable to read the v2 on-disk format and represent it as a tree-shaped index while we read it. Of course, there are things that are not as effective when reading from the flat v2 on-disk format (e.g. path limited reading will have to at least _scan_ the whole thing, even though it may process only the entries that match the pathspec) compared to reading from a tree-shaped on-disk format, but I doubt that the difference between the cost of reading into a flat array and the cost of reading and forming whatever non-flat data structure you seem to think is better is so big that it would negate the benefit of using a better in-core structure.

> This might not be the best way forward as v2 incompatible features
> (like keeping empty directories in index, what else?) may never come
> until v2 is deprecated.

I do not think "empty directories" matter to begin with, but even if it did, I do not think v2 is inherently incapable of being enhanced to record one if you really wanted to. Either you come up with a new "mode" bits and add it as a regular cache entry, or record the fact that there is a directory in a new index extension.

The real issue to solve is to decide what semantics you want (e.g. What to do when you earlier have added an empty directory, added a file in it and then removed the file, making it empty again? What if that happened during a merge?), to verify the semantics you define are sane, to add "keep_empty_directory()" function to read-cache.c, and to sprinkle callers to the API function as needed.

These have to be done regardless of the actual on-disk format.
Previous: Nguyen Thai Ngoc DuyNext: Nguyen Thai Ngoc Duy
Message 51 of 59 in “Introduce index file format version 5”
  1. 0/16 Introduce index file format version 5Thomas Gummerer, Aug 5, 2012
  2. 01/16 Modify cache_header to prepare for other index formatsThomas Gummerer, Aug 5, 2012
  3. Junio C HamanoAug 6, 2012
  4. Thomas GummererAug 7, 2012
  5. Junio C HamanoAug 7, 2012
  6. 02/16 Modify read functions to prepare for other index formatsThomas Gummerer, Aug 5, 2012
  7. 03/16 Modify match_stat_basic to prepare for other index formatsThomas Gummerer, Aug 5, 2012
  8. 04/16 Modify write functions to prepare for other index formatsThomas Gummerer, Aug 5, 2012
  9. Junio C HamanoAug 6, 2012
  10. Thomas GummererAug 7, 2012
  11. 05/16 t2104: Don't fail for index versions other than [23]Thomas Gummerer, Aug 5, 2012
  12. Junio C HamanoAug 6, 2012
  13. 06/16 t3700: sleep for 1 second, to avoid interfering with the racy codeThomas Gummerer, Aug 5, 2012
  14. Junio C HamanoAug 6, 2012
  15. Thomas GummererAug 7, 2012
  16. Junio C HamanoAug 8, 2012
  17. Junio C HamanoAug 8, 2012
  18. Thomas GummererAug 9, 2012
  19. Junio C HamanoAug 9, 2012
  20. Thomas GummererAug 9, 2012
  21. 07/16 Add documentation of the index-v5 file formatThomas Gummerer, Aug 5, 2012
  22. 08/16 Make in-memory format aware of stat_crcThomas Gummerer, Aug 5, 2012
  23. Junio C HamanoAug 6, 2012
  24. Thomas GummererAug 7, 2012
  25. 09/16 Read index-v5Thomas Gummerer, Aug 5, 2012
  26. Junio C HamanoAug 6, 2012
  27. Thomas GummererAug 8, 2012
  28. Junio C HamanoAug 8, 2012
  29. Thomas GummererAug 8, 2012
  30. Junio C HamanoAug 8, 2012
  31. 10/16 Read resolve-undo dataThomas Gummerer, Aug 5, 2012
  32. Junio C HamanoAug 6, 2012
  33. Thomas GummererAug 7, 2012
  34. 11/16 Read cache-tree in index-v5Thomas Gummerer, Aug 5, 2012
  35. 12/16 Write index-v5Thomas Gummerer, Aug 5, 2012
  36. 13/16 Write index-v5 cache-tree dataThomas Gummerer, Aug 5, 2012
  37. 14/16 Write resolve-undo data for index-v5Thomas Gummerer, Aug 5, 2012
  38. 15/16 update-index.c: add a force-rewrite optionThomas Gummerer, Aug 5, 2012
  39. Junio C HamanoAug 6, 2012
  40. Thomas GummererAug 8, 2012
  41. 16/16 p0002-index.sh: add perf test for the index formatsThomas Gummerer, Aug 5, 2012
  42. Nguyễn Thái Ngọc DuyAug 6, 2012
  43. 1/2 Move index v2 specific code out of read-cacheNguyễn Thái Ngọc Duy, Aug 6, 2012
  44. 2/2 Add index-v5Nguyễn Thái Ngọc Duy, Aug 6, 2012
  45. Robin RosenbergAug 7, 2012
  46. Thomas GummererAug 8, 2012
  47. Junio C HamanoAug 6, 2012
  48. Thomas GummererAug 6, 2012
  49. Junio C HamanoAug 6, 2012
  50. Nguyen Thai Ngoc DuyAug 7, 2012
  51. Junio C HamanoAug 8, 2012
  52. Nguyen Thai Ngoc DuyAug 8, 2012
  53. Junio C HamanoAug 8, 2012
  54. Nguyen Thai Ngoc DuyAug 9, 2012
  55. Thomas RastAug 7, 2012
  56. Junio C HamanoAug 7, 2012
  57. Thomas RastAug 8, 2012
  58. Junio C HamanoAug 8, 2012
  59. Nguyen Thai Ngoc DuyAug 8, 2012

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.