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

Re: [PATCH 2/2] sha1_file: only freshen packs once per run

From
SSStefan Saasen <ssaasen@atlassian.com>
Date
Apr 21, 2015, 00:45 UTC
Message-ID
<CADoxLGMrWXn+3qsGr2dYaDMmzLTKDW6wDthH6z1a_whOch9qPw@mail.gmail.com>
In-Reply-To
<20150420195500.GB15760@peff.net>
I can confirm that this patch is equivalent to the previous one.

https://bitbucket.org/snippets/ssaasen/9AXg shows both the timing and the NFS stats showing the effect of applying this patch.

Thanks for the fix Jeff!

Cheers, Stefan

On 21 April 2015 at 05:55, Jeff King <peff@peff.net> wrote:
Show 63 quoted lines
> Since 33d4221 (write_sha1_file: freshen existing objects,
> 2014-10-15), we update the mtime of existing objects that we
> would have written out (had they not existed). For the
> common case in which many objects are packed, we may update
> the mtime on a single packfile repeatedly. This can result
> in a noticeable performance problem if calling utime() is
> expensive (e.g., because your storage is on NFS).
>
> We can fix this by keeping a per-pack flag that lets us
> freshen only once per program invocation.
>
> An alternative would be to keep the packed_git.mtime flag up
> to date as we freshen, and freshen only once every N
> seconds. In practice, it's not worth the complexity. We are
> racing against prune expiration times here, which inherently
> must be set to accomodate reasonable program running times
> (because they really care about the time between an object
> being written and it becoming referenced, and the latter is
> typically the last step a program takes).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Hopefully I didn't botch the flag logic again. :) I tested with "strace
> -c" myself this time, so I think it is good.
>
>  cache.h     | 1 +
>  sha1_file.c | 9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/cache.h b/cache.h
> index 3d3244b..72c6888 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1174,6 +1174,7 @@ extern struct packed_git {
>         int pack_fd;
>         unsigned pack_local:1,
>                  pack_keep:1,
> +                freshened:1,
>                  do_not_close:1;
>         unsigned char sha1[20];
>         /* something like ".git/objects/pack/xxxxx.pack" */
> diff --git a/sha1_file.c b/sha1_file.c
> index 822aaef..26b9b2b 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2999,7 +2999,14 @@ static int freshen_loose_object(const unsigned char *sha1)
>  static int freshen_packed_object(const unsigned char *sha1)
>  {
>         struct pack_entry e;
> -       return find_pack_entry(sha1, &e) && freshen_file(e.p->pack_name);
> +       if (!find_pack_entry(sha1, &e))
> +               return 0;
> +       if (e.p->freshened)
> +               return 1;
> +       if (!freshen_file(e.p->pack_name))
> +               return 0;
> +       e.p->freshened = 1;
> +       return 1;
>  }
>
>  int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
> --
> 2.4.0.rc2.384.g7297a4a
Previous: Jeff KingNext: Junio C Hamano
Message 9 of 19 in “[BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects”
  1. Stefan SaasenApr 17, 2015
  2. Jeff KingApr 17, 2015
  3. Junio C HamanoApr 17, 2015
  4. Stefan SaasenApr 18, 2015
  5. Jeff KingApr 20, 2015
  6. 1/2 sha1_file: freshen pack objects before looseJeff King, Apr 20, 2015
  7. Stefan SaasenApr 21, 2015
  8. 2/2 sha1_file: only freshen packs once per runJeff King, Apr 20, 2015
  9. Stefan SaasenApr 21, 2015
  10. Junio C HamanoApr 20, 2015
  11. Jeff KingApr 20, 2015
  12. Junio C HamanoApr 20, 2015
  13. Jeff KingApr 20, 2015
  14. Stefan SaasenApr 21, 2015
  15. Junio C HamanoApr 21, 2015
  16. Junio C HamanoApr 21, 2015
  17. Stefan SaasenApr 22, 2015
  18. Junio C HamanoApr 22, 2015
  19. Stefan SaasenApr 22, 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.