From: Junio C Hamano <gitster@pobox•com>
To: Karthik Nayak <karthik.188@gmail•com>
Cc: git@vger•kernel.org, christian.couder@gmail•com,
Matthieu.Moy@grenoble-inp•fr
Subject: Re: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public
Date: Mon, 01 Jun 2015 07:53:51 -0700 [thread overview]
Message-ID: <xmqqvbf78oow.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <556B6234.2010809@gmail.com> (Karthik Nayak's message of "Mon, 01 Jun 2015 01:04:12 +0530")
Karthik Nayak <karthik.188@gmail•com> writes:
> Could be achieved using a simple wrapper around 'filter_refs()'
> something like this perhaps.
>
> int filter_refs_with_pattern(struct ref_array *ref, int
> (*for_each_ref_fn)(each_ref_fn, void *), char **patterns)
> {
> int i;
> struct ref_filter_cbdata data;
> data.filter.name_patterns = patterns;
> filter_refs(for_each_ref_fn, &data);
I presume that this is
filter_refs(&refs, for_each_ref_fn, &data);
as you would need to have some way to get the result back ;-)
> refs->nr = data.array.nr;
> for(i = 0; i < refs->nr; i++) {
> /* copy over the refs */
> }
> return 0;
> }
>
> Is this on the lines of what you had in mind? If it is, than I could
> just create a new patch which would make ref_filter_handler() private
> and introduce filter_refs() as shown.
Yeah. Even though I suggested
filter_refs(&for_each_ref, ...);
I actually would think the external interface should not mention
for_each_ref() like I did. The primary reason why I felt that it is
bad for the API to export a generic callback function the caller can
use to call for_each_ref() or for_each_rawref()" in the longer term
is because it forces us to always iterate all refs; for_each_ref()
does not know what the callback filter function wants to do. The
most common way to filter in the context of your GSoC project is "we
limit only to refs/heads/*, and then we may also filter by other
criteria" (that is "git branch" "-l" or possibly with "--contains",
etc.), and it is very wasteful for that codepath to allow
for_each_ref() to even enumerate and feed all refs outside the
refs/heads/* area to your callback, which would involve reading all
entries in packed-refs (which is a fixed cost so not an overhead)
and then reading everything in .git/refs/* (which is an overhead we
could and should avoid when we know we are only interested in the
branches that live in refs/heads*).
Your first implementation of course can just call for_each_ref() or
for_each_rawref(), and at the end of GSoC, the code may still do so.
But by keeping the external interface free of for_each_ref(), you
could later optimize.
And your above sample only takes for_each_ref_fn without exposing the
internal use of for_each_ref(), which is good.
next prev parent reply other threads:[~2015-06-01 14:54 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <5569EF77.4010300@gmail.com>
2015-05-30 17:53 ` [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref() Karthik Nayak
2015-05-31 2:58 ` Eric Sunshine
2015-05-31 8:11 ` Karthik Nayak
2015-05-31 17:34 ` Junio C Hamano
2015-05-31 17:48 ` Karthik Nayak
2015-05-31 20:39 ` Matthieu Moy
2015-06-01 14:39 ` Junio C Hamano
2015-05-30 17:53 ` [WIP/PATCH v4 2/8] for-each-ref: simplify code Karthik Nayak
2015-05-30 17:53 ` [WIP/PATCH v4 3/8] for-each-ref: rename 'refinfo' to 'ref_array_item' Karthik Nayak
2015-05-31 17:37 ` Junio C Hamano
2015-05-31 17:48 ` Karthik Nayak
2015-05-30 17:53 ` [WIP/PATCH v4 4/8] for-each-ref: introduce new structures for better organisation Karthik Nayak
2015-05-31 3:14 ` Eric Sunshine
2015-05-31 8:16 ` Karthik Nayak
2015-05-30 17:53 ` [WIP/PATCH v4 5/8] for-each-ref: introduce 'ref_filter_clear_data()' Karthik Nayak
2015-05-31 7:38 ` Christian Couder
2015-05-31 8:20 ` Karthik Nayak
2015-05-30 17:53 ` [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public Karthik Nayak
2015-05-31 3:21 ` Eric Sunshine
2015-05-31 8:16 ` Karthik Nayak
2015-05-31 8:04 ` Christian Couder
2015-05-31 8:11 ` Christian Couder
2015-05-31 9:17 ` Karthik Nayak
2015-05-31 14:03 ` Christian Couder
2015-05-31 15:30 ` Karthik Nayak
2015-06-01 6:38 ` Matthieu Moy
2015-06-01 19:28 ` Karthik Nayak
2015-05-31 17:54 ` Junio C Hamano
2015-05-31 17:48 ` Junio C Hamano
2015-05-31 19:34 ` Karthik Nayak
2015-06-01 14:53 ` Junio C Hamano [this message]
2015-06-01 19:35 ` Karthik Nayak
2015-05-30 17:53 ` [WIP/PATCH v4 7/8] ref-filter: move code from 'for-each-ref' Karthik Nayak
2015-05-30 17:53 ` [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h' Karthik Nayak
2015-05-31 3:43 ` Eric Sunshine
2015-05-31 8:19 ` Karthik Nayak
2015-05-31 8:29 ` Eric Sunshine
2015-05-31 9:12 ` Karthik Nayak
2015-05-31 20:46 ` Matthieu Moy
2015-05-31 20:50 ` Karthik Nayak
2015-05-31 22:34 ` Christian Couder
2015-06-01 6:47 ` Matthieu Moy
2015-05-31 8:20 ` Christian Couder
2015-05-31 9:16 ` Karthik Nayak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqvbf78oow.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Matthieu.Moy@grenoble-inp$(echo .)fr \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=karthik.188@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox