[v2,2/2] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily
diff mbox series

Message ID 20190118170549.30403-3-szeder.dev@gmail.com
State New
Headers show
Series
  • commit-graph: minor cleanup and optimization
Related show

Commit Message

SZEDER Gábor Jan. 18, 2019, 5:05 p.m. UTC
The optional 'Large Edge List' chunk of the commit graph file stores
parent information for commits with more than two parents.  Since the
chunk is optional, write_commit_graph() looks through all commits to
find those with more than two parents, and then writes the commit
graph file header accordingly, i.e. if there are no such commits, then
there won't be a 'Large Edge List' chunk written, only the three
mandatory chunks.

However, when it later comes to writing actual chunk data,
write_commit_graph() unconditionally invokes
write_graph_chunk_large_edges(), even when it was decided earlier that
that chunk won't be written.  Strictly speaking there is no bug here,
because write_graph_chunk_large_edges() won't write anything if it
doesn't find any commits with more than two parents, but then it
unnecessarily and in vain looks through all commits once again in
search for such commits.

Don't call write_graph_chunk_large_edges() when that chunk won't be
written to spare an unnecessary iteration over all commits.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 commit-graph.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Martin Ågren Jan. 19, 2019, 9:32 a.m. UTC | #1
On Fri, 18 Jan 2019 at 18:23, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> write_commit_graph() unconditionally invokes
> write_graph_chunk_large_edges(), even when it was decided earlier that
> that chunk won't be written.  Strictly speaking there is no bug here,
> because write_graph_chunk_large_edges() won't write anything if it

> Don't call write_graph_chunk_large_edges() when that chunk won't be
> written to spare an unnecessary iteration over all commits.

This commit message (including the one-line subject) needs some
s/_large_/_extra_/.

> -       write_graph_chunk_extra_edges(f, commits.list, commits.nr);
> +       if (num_extra_edges)
> +               write_graph_chunk_extra_edges(f, commits.list, commits.nr);

Martin

Patch
diff mbox series

diff --git a/commit-graph.c b/commit-graph.c
index 544d6b8b4a..bf9eb44ca9 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -940,7 +940,8 @@  void write_commit_graph(const char *obj_dir,
 	write_graph_chunk_fanout(f, commits.list, commits.nr);
 	write_graph_chunk_oids(f, GRAPH_OID_LEN, commits.list, commits.nr);
 	write_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr);
-	write_graph_chunk_extra_edges(f, commits.list, commits.nr);
+	if (num_extra_edges)
+		write_graph_chunk_extra_edges(f, commits.list, commits.nr);
 
 	close_commit_graph(the_repository);
 	finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC);