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: "Git List" <git@vger.kernel.org>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Christian Couder" <christian.couder@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [PATCH v2 4/9] object: add clear_commit_marks_all()
Date: Thu, 11 Jan 2018 19:57:42 +0100	[thread overview]
Message-ID: <a0ca86b8-e258-2588-1c99-a30e8e60fdbd@web.de> (raw)
In-Reply-To: <20180110075838.GD16315@sigill.intra.peff.net>

Am 10.01.2018 um 08:58 schrieb Jeff King:
> On Mon, Dec 25, 2017 at 06:44:58PM +0100, René Scharfe wrote:
> 
>> Add a function for clearing the commit marks of all in-core commit
>> objects.  It's similar to clear_object_flags(), but more precise, since
>> it leaves the other object types alone.  It still has to iterate through
>> them, though.
> 
> Makes sense.
> 
> Is it worth having:
> 
>    void clear_object_flags_from_type(int type, unsigned flags);
> 
> rather than having two near-identical functions? I guess we'd need some
> way of saying "all types" to reimplement clear_object_flags() as a
> wrapper (OBJ_NONE, I guess?).

I don't know if there is a demand.  Perhaps the two callers of
clear_object_flags() should be switched to clear_commit_marks_all()?
They look like they only care about commits as well.  Or is it safe to
stomp over the flags of objects of other types?  Then we'd only need
to keep clear_object_flags()..

> The run-time check is maybe a little bit slower in the middle of a tight
> loop, but I'm not sure it would matter much (I'd actually be curious if
> this approach is faster than the existing traversal code, too).

I don't know how to measure this properly.  With 100 runs each I get
this for the git repo and the silly test program below, which measures
the duration of the respective function call:

   mean        stddev method
   ----------- ------ ----------------------
   5.89763e+06 613106 clear_commit_marks
   2.72572e+06 507689 clear_commit_marks_all
   1.96582e+06 494753 clear_object_flags

So these are noisy numbers, but kind of in the expected range.

René

---
 Makefile                           |  1 +
 t/helper/test-clear-commit-marks.c | 67 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)
 create mode 100644 t/helper/test-clear-commit-marks.c

diff --git a/Makefile b/Makefile
index 1a9b23b679..7db2c6ca7f 100644
--- a/Makefile
+++ b/Makefile
@@ -648,6 +648,7 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-clear-commit-marks
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-config
 TEST_PROGRAMS_NEED_X += test-date
diff --git a/t/helper/test-clear-commit-marks.c b/t/helper/test-clear-commit-marks.c
new file mode 100644
index 0000000000..296ca0286f
--- /dev/null
+++ b/t/helper/test-clear-commit-marks.c
@@ -0,0 +1,67 @@
+#include "cache.h"
+#include "commit.h"
+#include "revision.h"
+
+static void start(struct timespec *start)
+{
+	clock_gettime(CLOCK_MONOTONIC, start);
+}
+
+static void print_duration(struct timespec *start)
+{
+	struct timespec end;
+	uint64_t d;
+	clock_gettime(CLOCK_MONOTONIC, &end);
+	d = end.tv_sec - start->tv_sec;
+	d *= 1000000000;
+	d += end.tv_nsec - start->tv_nsec;
+	printf("%lu", d);
+}
+
+int cmd_main(int argc, const char **argv)
+{
+	struct rev_info revs;
+	struct object_id oid;
+	struct commit *commit;
+	struct timespec ts;
+
+	setup_git_directory();
+
+	if (get_oid("HEAD", &oid))
+		die("No HEAD?");
+	commit = lookup_commit(&oid);
+	if (!commit)
+		die("HEAD is not a committish?");
+
+	init_revisions(&revs, NULL);
+	add_pending_object(&revs, &commit->object, "HEAD");
+	if (prepare_revision_walk(&revs))
+		die("revision walk setup failed");
+	while (get_revision(&revs))
+		; /* nothing */
+
+	if (argc != 2)
+		return 0;
+
+	if (!strcmp(argv[1], "clear_commit_marks")) {
+		start(&ts);
+		clear_commit_marks(commit, ALL_REV_FLAGS);
+		print_duration(&ts);
+	}
+
+	if (!strcmp(argv[1], "clear_commit_marks_all")) {
+		start(&ts);
+		clear_commit_marks_all(ALL_REV_FLAGS);
+		print_duration(&ts);
+	}
+
+	if (!strcmp(argv[1], "clear_object_flags")) {
+		start(&ts);
+		clear_object_flags(ALL_REV_FLAGS);
+		print_duration(&ts);
+	}
+
+	printf(" %s\n", argv[1]);
+
+	return 0;
+}
-- 
2.15.1

  reply	other threads:[~2018-01-11 18:58 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
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 [this message]
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=a0ca86b8-e258-2588-1c99-a30e8e60fdbd@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).