git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Christian Couder <christian.couder@gmail.com>
Date
Jul 15, 2021, 09:45 UTC
Message-ID
<CAP8UFD24X7UjXGKsRWr+f_xmX0x4EVDJHLBs2c1KhECb8-BnBw@mail.gmail.com>
In-Reply-To
<CAOLTT8Sa984Eo18QMBeGnMCX3_7sr+9qUYoAR4FS3UF6+CDtGw@mail.gmail.com>
On Thu, Jul 15, 2021 at 3:53 AM ZheNing Hu <adlternative@gmail.com> wrote:
>
> ZheNing Hu <adlternative@gmail.com> 于2021年7月15日周四 上午12:24写道:
> >
> > Junio C Hamano <gitster@pobox.com> 于2021年7月13日周二 上午4:38写道:
Show 39 quoted lines
> > > 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.

Is it possible to check early if any of the atoms that needs these copied data is specified, and if none of them is specified then to avoid the copies?

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

Are there way the more complex parsing could be avoided if it's not needed by the atoms that are actually used?

Show 8 quoted lines
> 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.
Ok, thanks for the details and your work on this performance issue!

I wonder if your patch series could be split, so that the early parts that add new atoms to ref-filter could be merged sooner?

Best, Christian.

Previous: ZheNing HuNext: ZheNing Hu
Message 25 of 52 in “[GSOC] cat-file: reuse ref-filter logic”
  1. 00/19 [GSOC] cat-file: reuse ref-filter logicZheNing Hu via GitGitGadget, Jul 12, 2021
  2. 02/19 cat-file: merge two block into oneZheNing Hu via GitGitGadget, Jul 12, 2021
  3. 01/19 cat-file: handle trivial --batch format with --batch-all-objectsZheNing Hu via GitGitGadget, Jul 12, 2021
  4. 03/19 [GSOC] ref-filter: add obj-type check in grab contentsZheNing Hu via GitGitGadget, Jul 12, 2021
  5. 07/19 [GSOC] ref-filter: add %(rest) atomZheNing Hu via GitGitGadget, Jul 12, 2021
  6. 08/19 [GSOC] ref-filter: pass get_object() return value to their callersZheNing Hu via GitGitGadget, Jul 12, 2021
  7. 06/19 [GSOC] ref-filter: use non-const ref_format in *_atom_parser()ZheNing Hu via GitGitGadget, Jul 12, 2021
  8. 04/19 [GSOC] ref-filter: add %(raw) atomZheNing Hu via GitGitGadget, Jul 12, 2021
  9. 05/19 [GSOC] ref-filter: --format=%(raw) re-support --perlZheNing Hu via GitGitGadget, Jul 12, 2021
  10. 09/19 [GSOC] ref-filter: introduce free_ref_array_item_value() functionZheNing Hu via GitGitGadget, Jul 12, 2021
  11. 11/19 [GSOC] ref-filter: modify the error message and value in get_objectZheNing Hu via GitGitGadget, Jul 12, 2021
  12. 10/19 [GSOC] ref-filter: introduce reject_atom()ZheNing Hu via GitGitGadget, Jul 12, 2021
  13. 12/19 [GSOC] cat-file: add has_object_file() checkZheNing Hu via GitGitGadget, Jul 12, 2021
  14. 13/19 [GSOC] cat-file: change batch_objects parameter nameZheNing Hu via GitGitGadget, Jul 12, 2021
  15. 15/19 [GSOC] cat-file: reuse err buf in batch_object_write()ZheNing Hu via GitGitGadget, Jul 12, 2021
  16. 16/19 [GSOC] cat-file: re-implement --textconv, --filters optionsZheNing Hu via GitGitGadget, Jul 12, 2021
  17. 14/19 [GSOC] cat-file: reuse ref-filter logicZheNing Hu via GitGitGadget, Jul 12, 2021
  18. Christian CouderJul 12, 2021
  19. Christian CouderJul 12, 2021
  20. ZheNing HuJul 12, 2021
  21. ZheNing HuJul 12, 2021
  22. Junio C HamanoJul 12, 2021
  23. ZheNing HuJul 14, 2021
  24. ZheNing HuJul 15, 2021
  25. Christian CouderJul 15, 2021
  26. ZheNing HuJul 15, 2021
  27. ZheNing HuJul 15, 2021
  28. 17/19 [GSOC] ref-filter: remove grab_oid() functionZheNing Hu via GitGitGadget, Jul 12, 2021
  29. 18/19 [GSOC] cat-file: create p1006-cat-file.shZheNing Hu via GitGitGadget, Jul 12, 2021
  30. 19/19 [GSOC] cat-file: use fast path when using default_formatZheNing Hu via GitGitGadget, Jul 12, 2021
  31. Christian CouderJul 12, 2021
  32. ZheNing HuJul 12, 2021
  33. Philip OakleyJul 12, 2021
  34. ZheNing HuJul 12, 2021
  35. 00/17 [GSOC] cat-file: reuse ref-filter logicZheNing Hu via GitGitGadget, Jul 15, 2021
  36. 01/17 [GSOC] ref-filter: add obj-type check in grab contentsZheNing Hu via GitGitGadget, Jul 15, 2021
  37. 02/17 [GSOC] ref-filter: add %(raw) atomZheNing Hu via GitGitGadget, Jul 15, 2021
  38. 03/17 [GSOC] ref-filter: --format=%(raw) re-support --perlZheNing Hu via GitGitGadget, Jul 15, 2021
  39. 04/17 [GSOC] ref-filter: use non-const ref_format in *_atom_parser()ZheNing Hu via GitGitGadget, Jul 15, 2021
  40. 06/17 [GSOC] ref-filter: pass get_object() return value to their callersZheNing Hu via GitGitGadget, Jul 15, 2021
  41. 07/17 [GSOC] ref-filter: introduce free_ref_array_item_value() functionZheNing Hu via GitGitGadget, Jul 15, 2021
  42. 05/17 [GSOC] ref-filter: add %(rest) atomZheNing Hu via GitGitGadget, Jul 15, 2021
  43. 09/17 [GSOC] ref-filter: modify the error message and value in get_objectZheNing Hu via GitGitGadget, Jul 15, 2021
  44. 10/17 [GSOC] cat-file: add has_object_file() checkZheNing Hu via GitGitGadget, Jul 15, 2021
  45. 11/17 [GSOC] cat-file: change batch_objects parameter nameZheNing Hu via GitGitGadget, Jul 15, 2021
  46. 12/17 [GSOC] cat-file: create p1006-cat-file.shZheNing Hu via GitGitGadget, Jul 15, 2021
  47. 08/17 [GSOC] ref-filter: add cat_file_mode to ref_formatZheNing Hu via GitGitGadget, Jul 15, 2021
  48. 15/17 [GSOC] cat-file: re-implement --textconv, --filters optionsZheNing Hu via GitGitGadget, Jul 15, 2021
  49. 14/17 [GSOC] cat-file: reuse err buf in batch_object_write()ZheNing Hu via GitGitGadget, Jul 15, 2021
  50. 13/17 [GSOC] cat-file: reuse ref-filter logicZheNing Hu via GitGitGadget, Jul 15, 2021
  51. 16/17 [GSOC] ref-filter: remove grab_oid() functionZheNing Hu via GitGitGadget, Jul 15, 2021
  52. 17/17 [GSOC] cat-file: use fast path when using default_formatZheNing Hu via GitGitGadget, Jul 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.