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

Re: [PATCH] [GSOC] ref-filter: use single strbuf for all output

From
ZheNing Hu <adlternative@gmail.com>
Date
Apr 6, 2021, 14:35 UTC
Message-ID
<CAOLTT8Q5kQh+8M3Ws4zjyid1+m3orBkootObxd0N2B6p8yNukg@mail.gmail.com>
In-Reply-To
<YGxpZ/vJ/MyS0tLB@coredump.intra.peff.net>
Jeff King <peff@peff.net> 于2021年4月6日周二 下午10:00写道:
Show 54 quoted lines
>
> On Tue, Apr 06, 2021 at 06:35:57PM +0800, ZheNing Hu wrote:
>
> > ZheNing Hu <adlternative@gmail.com> 于2021年4月6日周二 下午5:49写道:
> > > But this is the first time I use `t/perf/*` and there is a little problem.
> > > It seem like whatever I run single script like `sh ./p0007-write-cache.sh`
> > > or just `make` or `./run ${HOME}/git -- ./p0002-read-cache.sh` , these
> > > tests will fail.
> > >
> > It's because I don't have /usr/bin/time, solved after installation.
> > So best have this:
> >
> > --- a/t/perf/perf-lib.sh
> > +++ b/t/perf/perf-lib.sh
> > @@ -152,6 +152,10 @@ immediate=t
> >  # Perf tests require GNU time
> >  case "$(uname -s)" in Darwin) GTIME="${GTIME:-gtime}";; esac
> >  GTIME="${GTIME:-/usr/bin/time}"
> > +if ! test -f "$GTIME"
> > +then
> > +       error "command not found: "$GTIME""
> > +fi
>
> This patch would create problems when we expect to find the value of
> $GTIME in the $PATH e.g., you can see in the Darwin case it is set to
> just "gtime", not an absolute path).
>
> I am sympathetic to helping people see what's wrong, but I think in this
> case we're better off pointing people to using "-v". E.g.:
>
>   $ GTIME=pretend-we-do-not-have-gtime ./p0001-rev-list.sh
>   perf 1 - rev-list --all:
>   not ok 1 - rev-list --all
>   #
>   #             git rev-list --all >/dev/null
>   #
>
> Uh oh, that wasn't very informative. But how about this:
>
>   $ GTIME=pretend-we-do-not-have-gtime ./p0001-rev-list.sh -v
>   [...]
>   perf 1 - rev-list --all:
>   running:
>         git rev-list --all >/dev/null
>
>   ./p0001-rev-list.sh: 160: pretend-we-do-not-have-gtime: not found
>   not ok 1 - rev-list --all
>   #
>   #             git rev-list --all >/dev/null
>   #
>
> which I think makes it reasonably clear.
>
> -Peff

I just make a small suggestion. ;) You are right, "-v" is enough.

In addition, I found that the performance was basically unchanged after testing. It seems that this optimization is indeed too small, not as practical as in `cat-file`.

This shows that the performance bottleneck of `ref-filter` lies elsewhere. E.g. you mentioned "intermediate copies".

-- ZheNing Hu

Previous: Jeff KingNext: René Scharfe
Message 10 of 21 in “[GSOC] ref-filter: use single strbuf for all output”
  1. [GSOC] ref-filter: use single strbuf for all outputZheNing Hu via GitGitGadget, Apr 5, 2021
  2. Eric SunshineApr 5, 2021
  3. ZheNing HuApr 6, 2021
  4. Derrick StoleeApr 5, 2021
  5. ZheNing HuApr 6, 2021
  6. Jeff KingApr 5, 2021
  7. ZheNing HuApr 6, 2021
  8. ZheNing HuApr 6, 2021
  9. Jeff KingApr 6, 2021
  10. ZheNing HuApr 6, 2021
  11. René ScharfeApr 6, 2021
  12. ZheNing HuApr 7, 2021
  13. [GSOC] ref-filter: use single strbuf for all outputZheNing Hu via GitGitGadget, Apr 7, 2021
  14. Junio C HamanoApr 7, 2021
  15. ZheNing HuApr 8, 2021
  16. Jeff KingApr 7, 2021
  17. ZheNing HuApr 8, 2021
  18. Jeff KingApr 8, 2021
  19. ZheNing HuApr 8, 2021
  20. Jeff KingApr 8, 2021
  21. ZheNing HuApr 8, 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.