git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Jeff King <peff@peff.net>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Git List" <git@vger.kernel.org>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Christian Couder" <christian.couder@gmail.com>
Subject: Re: [PATCH] revision: introduce prepare_revision_walk_extended()
Date: Mon, 25 Dec 2017 18:36:48 +0100	[thread overview]
Message-ID: <7cb39d8a-2ef2-b7eb-d0cd-6931b2fda951@web.de> (raw)
In-Reply-To: <20171224142246.GA23648@sigill.intra.peff.net>

Am 24.12.2017 um 15:22 schrieb Jeff King:
> The single-traversal thing I suspect doesn't matter much in practice. In
> both cases if we would visit commit X twice, we'd immediately see on the
> second visit that it has already been cleared and not do anymore work.

Good point.  That makes clear_commit_marks_many() less useful than
advertised in e895cb5135, though.

>    Side note: Another question is whether it would simply be faster to
>    clear the flags for _all_ objects that we've touched in the current
>    process (we have clear_object_flags() for this already). Then we know
>    that we touch each one once, and we as a bonus we don't even have to
>    keep the previous tips. The downsides are:
> 
>      - if another traversal in the process looked at many objects, but
>        our current traversal looked at few, then we would examine more
>        objects than we need to (albeit with lower cost per object)
> 
>      - it's possible there's another traversal in the same process whose
>        flags we would want to have saved. I suspect such a setup is
>        broken already, though, unless there's a guarantee that the two
>        traversals don't overlap.

I thought about that nuclear option as well.  It might be a good idea
for code in cmd_* and similar leaf functions for cleaning up between
unrelated stages (e.g. between parts that had been separate external
git command calls before).  They probably only load potentially
interesting objects into memory and don't need to bother much about
interactions with other functions.

But clear_object_flags() makes me nervous because it clears the flags
of all kinds of objects, not just for commits, and I can't easily
convince myself that this is safe.  Adding a version that checks the
object type would be an easy way out.

René

  reply	other threads:[~2017-12-25 17:37 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-16 12:12 [PATCH] revision: introduce prepare_revision_walk_extended() René Scharfe
2017-12-17 10:20 ` Martin Ågren
2017-12-18 15:10 ` Jeff King
2017-12-18 19:18   ` René Scharfe
2017-12-19 11:49     ` Jeff King
2017-12-19 18:33       ` Junio C Hamano
2017-12-20 13:08         ` Jeff King
2017-12-21 18:41           ` René Scharfe
2017-12-24 14:22             ` Jeff King
2017-12-25 17:36               ` René Scharfe [this message]
2017-12-25 17:41 ` [PATCH v2 0/9] revision: get rid of the flag leak_pending René Scharfe
2017-12-25 17:43   ` [PATCH v2 1/9] commit: avoid allocation in clear_commit_marks_many() René Scharfe
2018-01-10  7:54     ` Jeff King
2017-12-25 17:44   ` [PATCH v2 2/9] commit: use clear_commit_marks_many() in remove_redundant() René Scharfe
2017-12-25 17:44   ` [PATCH v2 3/9] ref-filter: use clear_commit_marks_many() in do_merge_filter() René Scharfe
2017-12-25 17:44   ` [PATCH v2 4/9] object: add clear_commit_marks_all() René Scharfe
2018-01-10  7:58     ` Jeff King
2018-01-11 18:57       ` René Scharfe
2018-01-12 15:20         ` Jeff King
2017-12-25 17:45   ` [PATCH v2 5/9] bisect: avoid using the rev_info flag leak_pending René Scharfe
2018-01-10  8:07     ` Jeff King
2018-01-11 18:57       ` René Scharfe
2018-01-12 15:23         ` Jeff King
2017-12-25 17:46   ` [PATCH v2 6/9] bundle: " René Scharfe
2017-12-28 21:13     ` Junio C Hamano
2018-01-10  8:18     ` Jeff King
2017-12-25 17:47   ` [PATCH v2 7/9] checkout: " René Scharfe
2017-12-28 21:24     ` Junio C Hamano
2017-12-25 17:47   ` [PATCH v2 8/9] revision: remove the unused " René Scharfe
2017-12-25 17:48   ` [PATCH v2 9/9] commit: remove unused function clear_commit_marks_for_object_array() René Scharfe
2017-12-28 20:32   ` [PATCH v2 0/9] revision: get rid of the flag leak_pending Junio C Hamano
2018-01-10  8:20   ` Jeff King

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=7cb39d8a-2ef2-b7eb-d0cd-6931b2fda951@web.de \
    --to=l.s.r@web.de \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=peff@peff.net \
    /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).