From: Han Xin <hanxin.hx@bytedance.com>
To: git@vger.kernel.org
Cc: Han Xin <hanxin.hx@bytedance.com>,
xingxin.xx@bytedance.com, jonathantanmy@google.com,
derrickstolee@github.com, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 2/2] negotiator/skipping: fix some problems in mark_common()
Date: Wed, 26 Apr 2023 21:15:04 +0800 [thread overview]
Message-ID: <8b5c92a4d5d0927320f1e5fdb0136952f41be21c.1682513384.git.hanxin.hx@bytedance.com> (raw)
In-Reply-To: <cover.1682513384.git.hanxin.hx@bytedance.com>
The mark_common() method in negotiator/skipping.c was converted
from recursive to iterative in 4654134976f (negotiator/skipping:
avoid stack overflow, 2022-10-25), but there is some more work
to do:
1. prio_queue() should be used with clear_prio_queue(), otherwise there
will be a memory leak.
2. It does not do duplicate protection before prio_queue_put().
(The COMMON bit would work here, too.)
3. When it translated from recursive to iterative it kept "return"
statements that should probably be "continue" statements.
4. It does not attempt to parse commits, and instead returns
immediately when finding an unparsed commit. This is something
that it did in its original version, so maybe it is by design,
but it doesn't match the doc comment for the method.
Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Han Xin <hanxin.hx@bytedance.com>
---
negotiator/skipping.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/negotiator/skipping.c b/negotiator/skipping.c
index c7d6ab39bc..6a5450b460 100644
--- a/negotiator/skipping.c
+++ b/negotiator/skipping.c
@@ -85,29 +85,37 @@ static int clear_marks(const char *refname, const struct object_id *oid,
}
/*
- * Mark this SEEN commit and all its SEEN ancestors as COMMON.
+ * Mark this SEEN commit and all its parsed SEEN ancestors as COMMON.
*/
static void mark_common(struct data *data, struct commit *seen_commit)
{
struct prio_queue queue = { NULL };
struct commit *c;
+ if (seen_commit->object.flags & COMMON)
+ return;
+
prio_queue_put(&queue, seen_commit);
+ seen_commit->object.flags |= COMMON;
while ((c = prio_queue_get(&queue))) {
struct commit_list *p;
- if (c->object.flags & COMMON)
- return;
- c->object.flags |= COMMON;
+
if (!(c->object.flags & POPPED))
data->non_common_revs--;
if (!c->object.parsed)
- return;
+ continue;
for (p = c->parents; p; p = p->next) {
- if (p->item->object.flags & SEEN)
- prio_queue_put(&queue, p->item);
+ if (!(p->item->object.flags & SEEN) ||
+ (p->item->object.flags & COMMON))
+ continue;
+
+ p->item->object.flags |= COMMON;
+ prio_queue_put(&queue, p->item);
}
}
+
+ clear_prio_queue(&queue);
}
/*
--
2.40.0
next prev parent reply other threads:[~2023-04-26 13:15 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-24 2:23 [PATCH v1] negotiator/default.c: avoid stack overflow Han Xin
2023-04-24 14:44 ` Derrick Stolee
2023-04-25 3:02 ` [External] " Han Xin
2023-04-25 13:34 ` Derrick Stolee
2023-04-26 4:05 ` [PATCH v2 0/2] negotiator/default: " Han Xin
2023-04-26 4:05 ` [PATCH v2 1/2] " Han Xin
2023-04-26 11:13 ` Derrick Stolee
2023-04-26 11:40 ` [External] " Han Xin
2023-04-26 4:05 ` [PATCH v2 2/2] negotiator/skipping: fix some problems in mark_common() Han Xin
2023-04-26 11:08 ` Derrick Stolee
2023-04-26 11:55 ` [External] " Han Xin
2023-04-26 13:15 ` [PATCH v2 0/2] negotiator/default: avoid stack overflow Han Xin
2023-04-26 13:15 ` [PATCH v3 1/2] " Han Xin
2023-04-26 17:14 ` Junio C Hamano
2023-04-26 17:30 ` Derrick Stolee
2023-04-26 17:38 ` Junio C Hamano
2023-04-26 13:15 ` Han Xin [this message]
2023-05-01 22:11 ` [PATCH v2 0/2] " Junio C Hamano
2023-05-02 1:49 ` Derrick Stolee
2023-05-02 15:51 ` Junio C Hamano
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=8b5c92a4d5d0927320f1e5fdb0136952f41be21c.1682513384.git.hanxin.hx@bytedance.com \
--to=hanxin.hx@bytedance.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jonathantanmy@google.com \
--cc=xingxin.xx@bytedance.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.
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).