From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: gotwebd: avoid calloc/free per fcgi record
To: Omar Polo <op@omarpolo.com>
Cc: Stefan Sperling <stsp@stsp.name>, gameoftrees@openbsd.org
Date: Fri, 29 Jul 2022 16:11:34 -0600

On Fri, Jul 29, 2022 at 06:36:29PM +0200, Omar Polo wrote:
> Omar Polo <op@omarpolo.com> wrote:
> > Omar Polo <op@omarpolo.com> wrote:
> > > Omar Polo <op@omarpolo.com> wrote:
> > > > Stefan Sperling <stsp@stsp.name> wrote:
> > > > > On Fri, Jul 29, 2022 at 03:28:09PM +0200, Omar Polo wrote:
> > > > > > to send something to the browser we have to go through
> > > > > > fcgi_send_response.
> > > > > > 
> > > > > > diff below uses a static buffer in fcgi_send_response (now
> > > > > > send_response) to avoid dynamically allocating ~16K for each bit of the
> > > > > > reply.
> > > > > 
> > > > > Are you sure this approach is safe?
> > > > > Doesn't this introduce a risk where cross-request data leaks could
> > > > > become a potential issue?
> > > > 
> > > > I don't think; each request gets its own process so even in case of a
> > > > bug we could leak only data previously sent to the same client.
> > > 
> > > i misread the code badly.  the processes are preforked and handle one
> > > request per time, but then they are re-used for the next one.  thanks
> > > tracey@ for correcting me on irc.
> > > 
> > > > anyway, i wrote it with a buffer because i thought that the equivalent
> > > > with writev would be way more complex.  turns out, the version with an
> > > > iovec is not really worse (and saves a couple of memcpy too.)
> > > 
> > > fwiw, the writev approach saves us from allocating and from the risk of
> > > leaking data.
> > > 
> > > I wanted to get something like this in because we have a lot of calls to
> > > fcgi_gen_response with (very small) strings, and allocating ~16K for
> > > each of them is quite heavy.
> > > 
> > > > is this better?
> > 
> > i forgot to increment the iov base pointer on partial writes, here's a
> > revised diff:  (with also some other minor tweaks)
> 
> attaching the diff instead of inlining it to avoid issues with
> quoted-printable.
> 
> P.S.: anybody knows how to correctly inline diffs with mblaze?  I don't
> really like to add attachments with content-disposition=inline because
> they aren't included when replying (at least with the clients i tried)
> 

ok

-- 

Tracey Emery