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

Re: [PATCH 01/10] strbuf: Add strbuf_read_noblock

From
Stefan Beller <sbeller@google.com>
Date
Sep 17, 2015, 17:45 UTC
Message-ID
<CAGZ79kYnZr3nb_5n-5J0vCMi7xb91y-OkrAEq8-uH2PvzmkSmA@mail.gmail.com>
In-Reply-To
<20150917173536.GA28987@sigill.intra.peff.net>
On Thu, Sep 17, 2015 at 10:35 AM, Jeff King <peff@peff.net> wrote:
>
Show 6 quoted lines
>
> You _can_ loop on read until you hit EAGAIN, but I think in general you
> shouldn't; if you get a lot of input on this fd, you'll starve all of
> the other descriptors you're polling.  You're better off to read a
> finite amount from each descriptor, and then check again who is ready to
> read.

That's what I do with the current implementation. Except it's not as clear and concise as I patched it into the strbuf_read.

Show 22 quoted lines
>
> And then the return value becomes a no-brainer, because it's just the
> return value of read(). Either you got some data, you got EOF, or you
> get an error, which might be EAGAIN. You never have the case where you
> got some data, but then you also got EOF and EAGAIN, and the caller has
> to figure out which.
>
> So I think you really want something like:
>
>   ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)
>   {
>         ssize_t cnt;
>
>         strbuf_grow(hint ? hint : 8192);
>         cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
>         if (cnt > 0)
>                 strbuf_setlen(sb->len + cnt);
>         return cnt;
>   }
>
> (where I'm assuming xread passes us back EAGAIN; we could also replace
> it with read and loop on EINTR ourselves).

Yeah that's exactly what I am looking for (the hint may even be over engineered here, as I have no clue how much data comes back).

So I guess I could just use that new method now.
Show 8 quoted lines
> I actually wonder if callers who are _expecting_ non-blocking want to
> loop in strbuf_read() at all.
>
> strbuf_read() is really about reading to EOF, and growing the buffer to
> fit all of the input. But that's not at all what you want to do with
> non-blocking. There you believe for some reason that data may be
> available (probably due to poll), and you want to read one chunk of it,
> maybe act, and then go back to polling.

As a micro project (leftover bit maybe?): When strbuf_read is reading data out from a non blocking pipe, we're currently spinning (with the EAGAIN/EWOULDBLOCK). Introduce a call to poll to reduce the spinning.

>
> -Peff
Previous: Jeff KingNext: Jeff King
Message 12 of 37 in “fetch submodules in parallel and a preview on parallel "submodule update"”
  1. 00/10 fetch submodules in parallel and a preview on parallel "submodule update"Stefan Beller, Sep 17, 2015
  2. 01/10 strbuf: Add strbuf_read_noblockStefan Beller, Sep 17, 2015
  3. Junio C HamanoSep 17, 2015
  4. Jeff KingSep 17, 2015
  5. Junio C HamanoSep 17, 2015
  6. Stefan BellerSep 17, 2015
  7. Jeff KingSep 17, 2015
  8. Junio C HamanoSep 17, 2015
  9. Jeff KingSep 17, 2015
  10. Stefan BellerSep 17, 2015
  11. Jeff KingSep 17, 2015
  12. Stefan BellerSep 17, 2015
  13. Jeff KingSep 17, 2015
  14. Stefan BellerSep 17, 2015
  15. Junio C HamanoSep 17, 2015
  16. Junio C HamanoSep 17, 2015
  17. Jeff KingSep 17, 2015
  18. Stefan BellerSep 17, 2015
  19. 02/10 run-command: factor out return value computationStefan Beller, Sep 17, 2015
  20. Jeff KingSep 17, 2015
  21. 03/10 run-command: add an asynchronous parallel child processorStefan Beller, Sep 17, 2015
  22. Junio C HamanoSep 17, 2015
  23. Stefan BellerSep 17, 2015
  24. Junio C HamanoSep 18, 2015
  25. Stefan BellerSep 18, 2015
  26. 04/10 fetch_populated_submodules: use new parallel job processingStefan Beller, Sep 17, 2015
  27. 05/10 submodules: Allow parallel fetching, add tests and documentationStefan Beller, Sep 17, 2015
  28. 06/10 git submodule update: Redirect any output to stderrStefan Beller, Sep 17, 2015
  29. Eric SunshineSep 17, 2015
  30. Stefan BellerSep 17, 2015
  31. 07/10 git submodule update: pass --prefix only with a non empty prefixStefan Beller, Sep 17, 2015
  32. Eric SunshineSep 17, 2015
  33. 08/10 git submodule update: cmd_update_recursiveStefan Beller, Sep 17, 2015
  34. 09/10 git submodule update: cmd_update_recursiveStefan Beller, Sep 17, 2015
  35. Eric SunshineSep 17, 2015
  36. 10/10 git submodule update: cmd_update_fetchStefan Beller, Sep 17, 2015
  37. Jacob KellerSep 17, 2015

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.