Re: [PATCH 1/3] Git.pm: Add faculties to allow temp files to be cached
- From
- Eric Wong <normalperson@yhbt.net>
- Date
- Aug 12, 2008, 03:08 UTC
- Message-ID
- <20080812030809.GA14051@untitled>
- In-Reply-To
- <1218470035-13864-2-git-send-email-marcus@griep.us>
Marcus Griep <marcus@griep.us> wrote:
Show 13 quoted lines
> This patch offers a generic interface to allow temp files to be > cached while using an instance of the 'Git' package. If many > temp files are created and destroyed during the execution of a > program, this caching mechanism can help reduce the amount of > files created and destroyed by the filesystem. > > There are two methods offered for creating a new file: a no-lock and > a acquire-lock version. The no-lock version provides no > guarantee that a file is not in use or that the temp file may be > stolen by a subsequent request. The acquire-lock version provides a > weak guarantee that a temp file will not be stolen by subsequent > requests even from a no-lock request. If a file is locked when > another acquire request is made, a simple error is thrown.
I'm not sure if the no-lock version is worth the potential for buggy or dangerous code. I like this new idea of locking the files to prevent bugs.
Show 13 quoted lines
> +=item temp_release ( NAME [, BOOL] ) > + > +=item temp_release ( FILEHANDLE [, BOOL] ) > + > +Releases a lock acquired through C<temp_acquire()>. Can be called either with > +the C<NAME> mapping used when acquiring the temp file or with the C<FILEHANDLE> > +referencing a locked temp file. > + > +Warns if an attempt is made to release a file that is not locked. > + > +If called with C<BOOL> true, then the temp file will be truncated before being > +released. This can help to reduce disk I/O where the system is smart enough to > +detect the truncation while data is in the output buffers.
Always truncating on release makes the interface simpler. With locking, we can probably *only* truncate on release if you're that worried about the extra overhead :)
Show 11 quoted lines
> +=item temp_reset ( FILEHANDLE )
> +
> +Truncates and resets the position of the C<FILEHANDLE>. Uses C<sysseek>.
> +
> +=cut
> +
> +sub temp_reset {
> + my ($self, $temp_fd) = _maybe_self(@_);
> +
> + truncate $temp_fd, 0
> + or throw Error::Simple("couldn't truncate file");I would do a regular seek() here in addition to the sysseek() below. I am not certain one of the many userspace buffering layers Perl can potentially use doesn't do anything funky with its offset accounting.
> + sysseek $temp_fd, 0, SEEK_SET
> + or throw Error::Simple("couldn't seek to beginning of file");I would also put a tell() here after the sysseek and throw an error if it returns a non-zero value just in case. Yes, I'm really paranoid about this stuff and have a huge distrust of userspace I/O layers :)
-- Eric Wong