Re: [PATCH v3 2/2] find_pack_entry(): do not keep packed_git pointer locally
- From
Nicolas Pitre <nico@fluxnic.net>
- Date
- Feb 1, 2012, 16:02 UTC
- Message-ID
- <alpine.LFD.2.02.1202011100010.2759@xanadu.home>
- In-Reply-To
- <1328104135-475-2-git-send-email-pclouds@gmail.com>
On Wed, 1 Feb 2012, Nguyễn Thái Ngọc Duy wrote:
Show 11 quoted lines
> Commit f7c22cc (always start looking up objects in the last used pack > first - 2007-05-30) introduce a static packed_git* pointer as an > optimization. The kept pointer however may become invalid if > free_pack_by_name() happens to free that particular pack. > > Current code base does not access packs after calling > free_pack_by_name() so it should not be a problem. Anyway, move the > pointer out so that free_pack_by_name() can reset it to avoid running > into troubles in future. > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Acked-by: Nicolas Pitre <nico@fluxnic.net>
> Since Junio's already done the hard work. It'd be silly of me not to > take advantage and credit for free :)
Maybe a little "Thanks to Junio for code layout suggestions" in the commit message would give him some credit back.
> The new loop looks much better.
Indeed.
Show 65 quoted lines
> sha1_file.c | 27 +++++++++++++--------------
> 1 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index ff5bf42..ebe77b3 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -54,6 +54,8 @@ static struct cached_object empty_tree = {
> 0
> };
>
> +static struct packed_git *last_found_pack;
> +
> static struct cached_object *find_cached_object(const unsigned char *sha1)
> {
> int i;
> @@ -720,6 +722,8 @@ void free_pack_by_name(const char *pack_name)
> close_pack_index(p);
> free(p->bad_object_sha1);
> *pp = p->next;
> + if (last_found_pack == p)
> + last_found_pack = NULL;
> free(p);
> return;
> }
> @@ -2044,27 +2048,22 @@ static int find_pack_entry_1(const unsigned char *sha1,
>
> static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> {
> - static struct packed_git *last_found = (void *)1;
> struct packed_git *p;
>
> prepare_packed_git();
> if (!packed_git)
> return 0;
> - p = (last_found == (void *)1) ? packed_git : last_found;
>
> - do {
> - if (find_pack_entry_1(sha1, p, e)) {
> - last_found = p;
> - return 1;
> - }
> + if (last_found_pack && find_pack_entry_1(sha1, last_found_pack, e))
> + return 1;
>
> - if (p == last_found)
> - p = packed_git;
> - else
> - p = p->next;
> - if (p == last_found)
> - p = p->next;
> - } while (p);
> + for (p = packed_git; p; p = p->next) {
> + if (p == last_found_pack || !find_pack_entry_1(sha1, p, e))
> + continue;
> +
> + last_found_pack = p;
> + return 1;
> + }
> return 0;
> }
>
> --
> 1.7.8.36.g69ee2
>