{"thread":{"id":"12674","subject":"[PATCH] pack-objects: proper pack time stamping with --max-pack-size","startedAt":"2008-03-13T18:59:29Z","lastAt":"2008-03-14T05:13:04Z","messageCount":5,"participants":["Nicolas Pitre","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"71984","messageId":"alpine.LFD.1.00.0803131440590.2947@xanadu.home","threadId":"12674","inReplyTo":null,"subject":"[PATCH] pack-objects: proper pack time stamping with --max-pack-size","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-03-13T18:59:29Z","receivedAt":"2008-03-13T18:59:29Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Runtime pack access is done in the pack file mtime order since recent \npacks are more likely to contain frequently used objects than old packs.\nHowever the --max-pack-size option can produce multiple packs with mtime \nin the reversed order as newer objects are always written first.\n\nLet's modify mtime of later pack files (when any) so they appear older \nthan preceding ones when a repack creates multiple packs.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex f504cff..4c2ed70 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -17,6 +17,8 @@\n #include \"progress.h\"\n #include \"refs.h\"\n \n+#include <utime.h>\n+\n #ifdef THREADED_DELTA_SEARCH\n #include \"thread-utils.h\"\n #include <pthread.h>\n@@ -454,6 +456,7 @@ static void write_pack_file(void)\n \tstruct pack_header hdr;\n \tint do_progress = progress >> pack_to_stdout;\n \tuint32_t nr_remaining = nr_result;\n+\ttime_t last_mtime = 0;\n \n \tif (do_progress)\n \t\tprogress_state = start_progress(\"Writing objects\", nr_result);\n@@ -504,6 +507,7 @@ static void write_pack_file(void)\n \n \t\tif (!pack_to_stdout) {\n \t\t\tmode_t mode = umask(0);\n+\t\t\tstruct stat st;\n \t\t\tchar *idx_tmp_name, tmpname[PATH_MAX];\n \n \t\t\tumask(mode);\n@@ -511,6 +515,7 @@ static void write_pack_file(void)\n \n \t\t\tidx_tmp_name = write_idx_file(NULL, written_list,\n \t\t\t\t\t\t      nr_written, sha1);\n+\n \t\t\tsnprintf(tmpname, sizeof(tmpname), \"%s-%s.pack\",\n \t\t\t\t base_name, sha1_to_hex(sha1));\n \t\t\tif (adjust_perm(pack_tmp_name, mode))\n@@ -519,6 +524,28 @@ static void write_pack_file(void)\n \t\t\tif (rename(pack_tmp_name, tmpname))\n \t\t\t\tdie(\"unable to rename temporary pack file: %s\",\n \t\t\t\t    strerror(errno));\n+\n+\t\t\t/*\n+\t\t\t * Packs are runtime accessed in their mtime\n+\t\t\t * order since newer packs are more likely to contain\n+\t\t\t * younger objects.  So if we are creating multiple\n+\t\t\t * packs then we should modify the mtime of later ones\n+\t\t\t * to preserve this property.\n+\t\t\t */\n+\t\t\tif (stat(tmpname, &st) < 0) {\n+\t\t\t\twarning(\"failed to stat %s: %s\",\n+\t\t\t\t\ttmpname, strerror(errno));\n+\t\t\t} else if (!last_mtime) {\n+\t\t\t\tlast_mtime = st.st_mtime;\n+\t\t\t} else {\n+\t\t\t\tstruct utimbuf utb;\n+\t\t\t\tutb.actime = st.st_atime;\n+\t\t\t\tutb.modtime = --last_mtime;\n+\t\t\t\tif (utime(tmpname, &utb) < 0)\n+\t\t\t\t\twarning(\"failed utime() on %s: %s\",\n+\t\t\t\t\t\ttmpname, strerror(errno));\n+\t\t\t}\n+\n \t\t\tsnprintf(tmpname, sizeof(tmpname), \"%s-%s.idx\",\n \t\t\t\t base_name, sha1_to_hex(sha1));\n \t\t\tif (adjust_perm(idx_tmp_name, mode))\n@@ -527,6 +554,7 @@ static void write_pack_file(void)\n \t\t\tif (rename(idx_tmp_name, tmpname))\n \t\t\t\tdie(\"unable to rename temporary index file: %s\",\n \t\t\t\t    strerror(errno));\n+\n \t\t\tfree(idx_tmp_name);\n \t\t\tfree(pack_tmp_name);\n \t\t\tputs(sha1_to_hex(sha1));\n"},{"id":"71993","messageId":"7vzlt2wdb4.fsf@gitster.siamese.dyndns.org","threadId":"12674","inReplyTo":"alpine.LFD.1.00.0803131440590.2947@xanadu.home","subject":"Re: [PATCH] pack-objects: proper pack time stamping with --max-pack-size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-13T20:59:11Z","receivedAt":"2008-03-13T20:59:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> Runtime pack access is done in the pack file mtime order since recent \n> packs are more likely to contain frequently used objects than old packs.\n> However the --max-pack-size option can produce multiple packs with mtime \n> in the reversed order as newer objects are always written first.\n>\n> Let's modify mtime of later pack files (when any) so they appear older \n> than preceding ones when a repack creates multiple packs.\n\nIt is a very good companion to f7c22cc (always start looking up objects in\nthe last used pack first, May 30, 2007) after 10 months ;-)\n"},{"id":"72024","messageId":"7vmyp1vsa7.fsf@gitster.siamese.dyndns.org","threadId":"12674","inReplyTo":"alpine.LFD.1.00.0803131440590.2947@xanadu.home","subject":"Re: [PATCH] pack-objects: proper pack time stamping with --max-pack-size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-14T04:33:20Z","receivedAt":"2008-03-14T04:33:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> Runtime pack access is done in the pack file mtime order since recent \n> packs are more likely to contain frequently used objects than old packs.\n> However the --max-pack-size option can produce multiple packs with mtime \n> in the reversed order as newer objects are always written first.\n>\n> Let's modify mtime of later pack files (when any) so they appear older \n> than preceding ones when a repack creates multiple packs.\n>\n> Signed-off-by: Nicolas Pitre <nico@cam.org>\n> ---\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index f504cff..4c2ed70 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -17,6 +17,8 @@\n>  #include \"progress.h\"\n>  #include \"refs.h\"\n>  \n> +#include <utime.h>\n> +\n\nHmmm.  Shouldn't this go to git-compat-util.h?\n"},{"id":"72026","messageId":"alpine.LFD.1.00.0803140035260.2947@xanadu.home","threadId":"12674","inReplyTo":"7vmyp1vsa7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] pack-objects: proper pack time stamping with --max-pack-size","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-03-14T04:39:02Z","receivedAt":"2008-03-14T04:39:02Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 13 Mar 2008, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@cam.org> writes:\n> \n> > Runtime pack access is done in the pack file mtime order since recent \n> > packs are more likely to contain frequently used objects than old packs.\n> > However the --max-pack-size option can produce multiple packs with mtime \n> > in the reversed order as newer objects are always written first.\n> >\n> > Let's modify mtime of later pack files (when any) so they appear older \n> > than preceding ones when a repack creates multiple packs.\n> >\n> > Signed-off-by: Nicolas Pitre <nico@cam.org>\n> > ---\n> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> > index f504cff..4c2ed70 100644\n> > --- a/builtin-pack-objects.c\n> > +++ b/builtin-pack-objects.c\n> > @@ -17,6 +17,8 @@\n> >  #include \"progress.h\"\n> >  #include \"refs.h\"\n> >  \n> > +#include <utime.h>\n> > +\n> \n> Hmmm.  Shouldn't this go to git-compat-util.h?\n\nMaybe... if you say so.\n\nThe whole header file arrangement logic is somewhat escaping my mind, so \nI simply decided to do the same as in test-chmtime.c.\n\n\nNicolas\n"},{"id":"72036","messageId":"7vlk4lubvj.fsf@gitster.siamese.dyndns.org","threadId":"12674","inReplyTo":"alpine.LFD.1.00.0803140035260.2947@xanadu.home","subject":"Re: [PATCH] pack-objects: proper pack time stamping with --max-pack-size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-14T05:13:04Z","receivedAt":"2008-03-14T05:13:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n>> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n>> > index f504cff..4c2ed70 100644\n>> > --- a/builtin-pack-objects.c\n>> > +++ b/builtin-pack-objects.c\n>> > @@ -17,6 +17,8 @@\n>> >  #include \"progress.h\"\n>> >  #include \"refs.h\"\n>> >  \n>> > +#include <utime.h>\n>> > +\n>> \n>> Hmmm.  Shouldn't this go to git-compat-util.h?\n>\n> Maybe... if you say so.\n>\n> The whole header file arrangement logic is somewhat escaping my mind, so \n> I simply decided to do the same as in test-chmtime.c.\n\nThe general idea is to isolate the place system headers are included at,\nso that we have to worry about the inclusion and feature-macro definition\norder at only one place (see for example the hack about _ALL_SOURCE to\nwork around AIX 5.3, yuck).\n"}]}