Re: [PATCH v4 03/16] index-helper: new daemon for caching index and related stuff

[David Turner <dturner@twopensource.com>]

On Wed, 2016-04-13 at 20:43 +0700, Duy Nguyen wrote:
> On Wed, Apr 13, 2016 at 7:32 AM, David Turner <
> dturner@twopensource.com> wrote:
> > +NOTES
> > +-----
> > +
> > +$GIT_DIR/index-helper.path is a symlink
> 
> In multiple worktree context, this file will be per-worktree. So we
> have one daemon per worktree. I think that's fine.
> 
> > to a directory in $TMPDIR
> > +containing a Unix domain socket called 's' that the daemon reads
> > +commands from.
> 
> Oops. I stand corrected, now it's one daemon per repository...
> Probably good to hide the socket path in $GIT_DIR though, people may
> protect it with dir permission of one of ancestor directories.

I'm not sure I understand what you're saying here.  It should be one
daemon per worktree, I think.  And as far as I know, it is.  

Socket paths must be short (less than 104 chars on Mac).  That's why I
do the weird symlink-to-tmpdir thing.

> > The directory will also contain files named
> > +"git-index-<SHA1>".  These are used as backing stores for shared
> > +memory.  Normally the daemon will clean up these files when it
> > exits
> > +or when they are no longer relevant.  But if it crashes, some
> > objects
> > +could remain there and they can be safely deleted with "rm"
> > +command.
> 
> Alternatively, we could store all these in $GIT_DIR/helper or
> something and clean up automatically when index-helper starts. But I
> guess at least with TMPDIR we have a chance to put them on tmpfs.
> > +#define UNIX_PATH_MAX 92
> > +#endif
> > +
> 
> This looks like dead code (or at least not used in this patch).

Yep, thanks.

> > +       fd = unix_stream_connect(socket_path);
> > +       if (refresh_cache) {
> > +               ret = write_in_full(fd, "refresh", 8) != 8;
> 
> Since we've moved to unix socket and had bidirectional communication,
> it's probably a good idea to read an "ok" back, giving index-helper
> time to prepare the cache. As I recall the last discussion with
> Johannes, missing a cache here when the index is around 300MB could
> hurt more than wait patiently once and have it ready next time.

It is somewhat slower to wait for the daemon (which requires a disk
load + a memcpy) than it is to just load it ourselves (which is just a
disk load).