Re: [PATCH 14/19] [GSOC] cat-file: reuse ref-filter logic

From: ZheNing Hu <adlternative@gmail.com>
Date: Jul 15, 2021, 01:53 UTC
Message-ID: <CAOLTT8Sa984Eo18QMBeGnMCX3_7sr+9qUYoAR4FS3UF6+CDtGw@mail.gmail.com>
In-Reply-To: <CAOLTT8RR4+tUuT2yc2PDL9NwCburW8bM_Sht6nhKJ_fYV8fGsQ@mail.gmail.com>

ZheNing Hu <adlternative@gmail.com> 于2021年7月15日周四 上午12:24写道：

Show 41 quoted lines

>
> Junio C Hamano <gitster@pobox.com> 于2021年7月13日周二 上午4:38写道：
>
> >
> > Christian Couder <christian.couder@gmail.com> writes:
> >
> > More importantly, why is such a fast-path even needed?  Isn't it a
> > sign that the ref-filter implementation is eating more cycles than
> > it should for given set of placeholders?  Do we know where the extra
> > cycles goes?
> >
> > I find it somewhat alarming if we are talking about "fast-path"
> > workaround before understanding why we are seeing slowdown in the
> > first place.
>
> There is no complete conclusion yet, but I try to use time and hyperfine test
> for these commits (t/perf/* is not accurate enough):
>
> ----------------------------------------------------------------------------------------------------------------------------
> |                        subject                                  |
> --batch-check (using hyperfine) |   --batch(using time) |
> ----------------------------------------------------------------------------------------------------------------------------
> |[GSOC] cat-file: use fast path when using default_format         |
>         700ms                |          25.450s      |
> ----------------------------------------------------------------------------------------------------------------------------
> |[GSOC] cat-file: re-implement --textconv, --filters options      |
>         790ms                |          29.933s      |
> ----------------------------------------------------------------------------------------------------------------------------
> |[GSOC] cat-file: reuse err buf in batch_object_write()           |
>         770ms                |          29.153s      |
> ----------------------------------------------------------------------------------------------------------------------------
> |[GSOC] cat-file: reuse ref-filter logic                          |
>         780ms                |          29.412s      |
> ----------------------------------------------------------------------------------------------------------------------------
> |The third batch (upstream/master)                                |
>         640ms                |          26.025s      |
> ----------------------------------------------------------------------------------------------------------------------------
>
> I think we their cost is indeed from "[GSOC] cat-file: reuse ref-filter logic".
> But what causes the loss of performance needs further analysis.
>

Now I think: There are three main reasons why the performance of cat-file --batch deteriorates after refactor.

1. Too many copies are used in ref-filter and we cannot avoid these copies
easily because ref-filter needs these copied data to implement atoms %(if),
%(else), %(end)... and the --sort option. The original cat-file
--batch only needs
to output the data to the final string. Its copy times are relatively small.

2. More complex data structure and parsing process are used in ref-filter.
This is why it can provide more and more useful atoms. Therefore, I think the
performance degradation that occurs here is normal.

3. As Ævar Arnfjörð Bjarmason mentioned, oid_object_info_extend() was used
twice in get_object() before. oid_object_info_extend() is the hot
path, we should
try to avoid calling it, So in last version of  "[GSOC] cat-file:
re-implement --textconv,
--filters options", I make the unified processing of --textconv and
--filter avoid calling
oid_object_info_extend() twice.

Thanks. -- ZheNing Hu