From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/1] diffcore: add a filter to find a specific blob
Date: Thu, 07 Dec 2017 13:40:17 -0800 [thread overview]
Message-ID: <xmqq8teecj3y.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqpo88m896.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Fri, 24 Nov 2017 16:43:49 +0900")
Junio C Hamano <gitster@pobox.com> writes:
After saying "Will merge to 'next'" in the recent "What's cooking"
report, I noticed that a few loose ends were never tied on this
topic.
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index dd0dba5b1d..252a21cc19 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -500,6 +500,10 @@ information.
>> --pickaxe-regex::
>> Treat the <string> given to `-S` as an extended POSIX regular
>> expression to match.
>> +--blobfind=<blob-id>::
>> + Restrict the output such that one side of the diff
>> + matches the given blob-id.
>> +
>> endif::git-format-patch[]
>
> Can we have a blank line between these enumerations to make the
> source easier to read? Thanks.
>
> ...
> So, we keep an unmerged pair, a pair that mentions a sought-blob on
> one side or the other side? I am not sure if we want to keep the
> unmerged pair for the purpose of this one.
>
>> + diff_free_filepair(p);
>> + q->queue[i] = NULL;
>> + c--;
>
> Also, if you are doing the in-place shrinking and have already
> introduced another counter 'j' that is initialized to 0, I think it
> makes more sense to do the shrinking in-place. 'i' will stay to be
> the source-scan pointer that runs 0 thru q->nr, while 'j' can be
> used in this loop (where you have 'continue') to move the current
> one that is determined to survive from q->queue[i] to q->queue[j++].
>
> Then you do not need 'c'; when the loop ends, 'j' would be the
> number of surviving entries and q->nr can be adjusted to it. Unlike
> the usual pattern taken by the other diffcore transformations where
> a new queue is populated and the old one discarded, this would leave
> the q->queue[] over-allocated, but I do not think it is too bad.
Here is to illustrate the last point. I still think we should keep
the unmerged entries for the purpose of blobfind but it should be
trivial to fix that.
diffcore-blobfind.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/diffcore-blobfind.c b/diffcore-blobfind.c
index 5d222fc336..bf63ba61dc 100644
--- a/diffcore-blobfind.c
+++ b/diffcore-blobfind.c
@@ -8,40 +8,31 @@
static void diffcore_filter_blobs(struct diff_queue_struct *q,
struct diff_options *options)
{
- int i, j = 0, c = q->nr;
+ int src, dst;
if (!options->blobfind)
BUG("blobfind oidset not initialized???");
- for (i = 0; i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
+ for (src = dst = 0; src < q->nr; src++) {
+ struct diff_filepair *p = q->queue[src];
if (DIFF_PAIR_UNMERGED(p) ||
(DIFF_FILE_VALID(p->one) &&
oidset_contains(options->blobfind, &p->one->oid)) ||
(DIFF_FILE_VALID(p->two) &&
- oidset_contains(options->blobfind, &p->two->oid)))
- continue;
-
- diff_free_filepair(p);
- q->queue[i] = NULL;
- c--;
- }
-
- /* Keep it sorted. */
- i = 0; j = 0;
- while (i < c) {
- while (!q->queue[j])
- j++;
- q->queue[i] = q->queue[j];
- i++; j++;
+ oidset_contains(options->blobfind, &p->two->oid))) {
+ q->queue[dst] = p;
+ dst++;
+ } else {
+ diff_free_filepair(p);
+ }
}
- q->nr = c;
-
- if (!c) {
+ if (!dst) {
free(q->queue);
DIFF_QUEUE_CLEAR(q);
+ } else {
+ q->nr = dst;
}
}
next prev parent reply other threads:[~2017-12-07 21:40 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-20 22:25 [PATCH 0/1] Teaching the diff machinery about blobfind [WAS: git describe <blob>] Stefan Beller
2017-11-20 22:25 ` [PATCH 1/1] diffcore: add a filter to find a specific blob Stefan Beller
2017-11-24 7:43 ` Junio C Hamano
2017-11-25 4:59 ` Junio C Hamano
2017-12-07 21:40 ` Junio C Hamano [this message]
-- strict thread matches above, loose matches on Subject: below --
2017-12-08 0:24 [PATCH 0/1] diffcore-blobfind Stefan Beller
2017-12-08 0:24 ` [PATCH 1/1] diffcore: add a filter to find a specific blob Stefan Beller
2017-12-08 9:34 ` Jeff King
2017-12-08 16:28 ` Ramsay Jones
2017-12-08 20:19 ` Jeff King
2017-12-08 20:39 ` Stefan Beller
2017-12-08 21:38 ` Jeff King
2017-12-08 15:04 ` Junio C Hamano
2017-12-08 17:21 ` Junio C Hamano
2017-12-08 21:11 ` Stefan Beller
2017-12-08 21:15 ` Junio C Hamano
2017-12-11 19:58 ` [PATCH 0/1] diff-core blobfind Stefan Beller
2017-12-11 19:58 ` [PATCH 1/1] diffcore: add a filter to find a specific blob Stefan Beller
2017-12-11 23:17 ` Junio C Hamano
2017-12-12 0:21 ` Stefan Beller
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq8teecj3y.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).