Am 19.09.19 um 23:47 schrieb SZEDER Gábor:
> At the beginning of the recursive name_rev() function it parses the
> commit it got as parameter, and returns early if the commit is older
> than a cutoff limit.
>
> Restructure this so the caller parses the commit and checks its date,
> and doesn't invoke name_rev() if the commit to be passed as parameter
> is older than the cutoff, i.e. both name_ref() before calling
> name_rev() and name_rev() itself as it iterates over the parent
> commits.
>
> This makes eliminating the recursion a bit easier to follow, and it
> will be moved back to name_rev() after the recursion is eliminated.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
> ---
> builtin/name-rev.c | 29 ++++++++++++++++-------------
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 42cea5c881..99643aa4dc 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -107,11 +107,6 @@ static void name_rev(struct commit *commit,
> struct commit_list *parents;
> int parent_number = 1;
>
> - parse_commit(commit);
> -
> - if (commit->date < cutoff)
> - return;
> -
> if (!create_or_update_name(commit, tip_name, taggerdate, generation,
> distance, from_tag))
> return;
> @@ -119,6 +114,12 @@ static void name_rev(struct commit *commit,
> for (parents = commit->parents;
> parents;
> parents = parents->next, parent_number++) {
> + struct commit *parent = parents->item;
> +
> + parse_commit(parent);
> + if (parent->date < cutoff)
> + continue;
> +
> if (parent_number > 1) {
> size_t len;
> char *new_name;
> @@ -131,11 +132,11 @@ static void name_rev(struct commit *commit,
> new_name = xstrfmt("%.*s^%d", (int)len, tip_name,
> parent_number);
>
The check now also skips this allocation for old commits...
> - name_rev(parents->item, new_name, taggerdate, 0,
> + name_rev(parent, new_name, taggerdate, 0,
> distance + MERGE_TRAVERSAL_WEIGHT,
> from_tag);
> } else {
> - name_rev(parents->item, tip_name, taggerdate,
> + name_rev(parent, tip_name, taggerdate,
> generation + 1, distance + 1,
> from_tag);
> }
> @@ -269,16 +270,18 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
> if (o && o->type == OBJ_COMMIT) {
> struct commit *commit = (struct commit *)o;
> int from_tag = starts_with(path, "refs/tags/");
> - const char *tip_name;
>
> if (taggerdate == TIME_MAX)
> taggerdate = commit->date;
> path = name_ref_abbrev(path, can_abbreviate_output);
> - if (deref)
> - tip_name = xstrfmt("%s^0", path);
> - else
> - tip_name = xstrdup(path);
> - name_rev(commit, tip_name, taggerdate, 0, 0, from_tag);
> + if (commit->date >= cutoff) {
> + const char *tip_name;
> + if (deref)
> + tip_name = xstrfmt("%s^0", path);
> + else
> + tip_name = xstrdup(path);
... and this allocation here as well. If this improves performance
in a meaningful way then perhaps it should be kept at this place?
And if it doesn't, then an additional allocation might not hurt much?
Just a thought, I still didn't measure..
> + name_rev(commit, tip_name, taggerdate, 0, 0, from_tag);
> + }
> }
> return 0;
> }
>
