[GSOC,v4,0/4] Move generation, graph_pos to a slab

Message

Abhishek Kumar June 17, 2020, 9:14 a.m. UTC

The struct commit is used in many contexts. However, members
`generation` and `graph_pos` are only used for commit graph related
operations and otherwise waste memory.

This wastage would have been more pronounced as we transition to
generation nuber v2, which uses 64-bit generation number instead of
current 32-bits.

While the overall test suite runs as fast as master
(series: 26m48s, master: 27m34s, faster by 2.87%), certain commands
like `git merge-base --is-ancestor` were slowed by 40% as discovered
by Szeder Gábor [1]. After minimizing commit-slab access, the slow down
persists but is closer to 20%.

Derrick Stolee believes the slow down is attributable to the underlying
algorithm rather than the slowness of commit-slab access [2] and we will
follow-up in a later series.

Abhishek Kumar (4):
  object: drop parsed_object_pool->commit_count
  commit-graph: introduce commit_graph_data_slab
  commit: move members graph_pos, generation to a slab
  commit-graph: minimize commit_graph_data_slab access

 alloc.c                         |  18 +++--
 alloc.h                         |   2 +-
 blame.c                         |   2 +-
 blob.c                          |   2 +-
 bloom.c                         |   7 +-
 builtin/commit-graph.c          |   2 +-
 builtin/fsck.c                  |   2 +-
 commit-graph.c                  | 130 ++++++++++++++++++++++++--------
 commit-graph.h                  |  10 +++
 commit-reach.c                  |  69 ++++++++++-------
 commit.c                        |  12 +--
 commit.h                        |   2 -
 contrib/coccinelle/commit.cocci |  18 +++++
 object.c                        |   4 +-
 object.h                        |   3 +-
 refs.c                          |   2 +-
 revision.c                      |  20 +++--
 t/helper/test-reach.c           |   2 +-
 tag.c                           |   2 +-
 tree.c                          |   2 +-
 20 files changed, 217 insertions(+), 94 deletions(-)

Comments

Derrick Stolee June 19, 2020, 1:59 p.m. UTC | #1

On 6/17/2020 5:14 AM, Abhishek Kumar wrote:
> The struct commit is used in many contexts. However, members
> `generation` and `graph_pos` are only used for commit graph related
> operations and otherwise waste memory.
> 
> This wastage would have been more pronounced as we transition to
> generation nuber v2, which uses 64-bit generation number instead of
> current 32-bits.

Thanks, Szeder (CC'd) for the quality review in the previous
versions. I manually built and tested all of the patches here
and verified they passed all tests.

I think this series is in good shape.

Thanks,
-Stolee

Junio C Hamano June 19, 2020, 5:44 p.m. UTC | #2

Derrick Stolee <[e-mail]> writes:

> On 6/17/2020 5:14 AM, Abhishek Kumar wrote:
>> The struct commit is used in many contexts. However, members
>> `generation` and `graph_pos` are only used for commit graph related
>> operations and otherwise waste memory.
>> 
>> This wastage would have been more pronounced as we transition to
>> generation nuber v2, which uses 64-bit generation number instead of
>> current 32-bits.
>
> Thanks, Szeder (CC'd) for the quality review in the previous
> versions. I manually built and tested all of the patches here
> and verified they passed all tests.
>
> I think this series is in good shape.

Thank you to all who are involved in this topic.  Looking good.