Re: [PATCH v2 4/9] object: add clear_commit_marks_all()

From

René Scharfe <l.s.r@web.de>

Date

Jan 11, 2018, 18:57 UTC

Message-ID

<a0ca86b8-e258-2588-1c99-a30e8e60fdbd@web.de>

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?).

> 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).

   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