> > > (2) Why should file_setaside mmap the file? I'd think that we'd want to > > > keep it as a file as long as possible to make it easier to use > > > sendfile()... what am I missing? > > > > We are going to be copying something. I figured mmap'ing the file would > > be a bit better, because we could write the file out. > > Why would we need to write the file out?
Sooner or later, we are going to be writing the file someplace. That could be the network, or it could be into memory to do some mucking with the data. Since we are either dup'ing it or mmap'ing it, I figured we could be conservative here and mmap the data. If we dup it here, and then have to read, then we have dup'ed and mmap'ed. This way, we only mmap the data. > > doesn't really matter. > > I thought it mattered because we prefer having a file descriptor in > core_output to having an mmap in core_output. I guess I'm missing > your point. We only prefer the file descriptor because that is the fastest way to send the data. Once we are setting the data aside we have lost the opportunity to send this as quickly as possible. I would prefer to ensure that we copy the data as infrequently as possible, which given the logic above should make sense. > > > (3) You don't really need to dup() the file, do you? You can palloc a new > > > apr_file_t in the requested pool and use apr_os_file_get() and > > > apr_os_file_put() to move the os file handle into it. mod_file_cache in > > > Apache does something like this. It should be cheaper to do this than to > > > do a full dup(), I think. > > > > The file was opened with the request->pool. If we just > > apr_os_file_get/put, we will still close the file when the request_pool is > > cleared. We have to dup, or the file won't be available to us, and the > > original bug will be back. > > Isn't it just a matter of killing the cleanup associated with one pool > and registering the cleanup with the new pool? That only handles the close. It doesn't handle the rest of the internals to the file descriptor. When we dup, we copy everything that is known about the file. If we just kill the cleanup and re-register it, we are losing a lot of information. It is okay for apr_os_get/put_file to lose that information, because they are meant to be used at boundary locations between APR code and non-APR code. For this application, losing that information is a bad idea IMO. Take a look at how much data we are copying in the dup function to see the information I am refering to. Ryan _______________________________________________________________________________ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 -------------------------------------------------------------------------------
