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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 20, 2016, 12:17 UTC
Message-ID: <alpine.DEB.2.20.1604201349590.2826@virtualbox>
In-Reply-To: <1461108489-29376-4-git-send-email-dturner@twopensource.com>

Hi Dave,

(apologies in advance if I may bring up anything that has been discussed in earlier iterations; I simply was too busy with the rebase--helper project to even look.)

On Tue, 19 Apr 2016, David Turner wrote:

Show 5 quoted lines

> Shared memory is done by storing files in a per-repository temporary
> directory.  This is more portable than shm (which requires
> posix-realtime and has various quirks on OS X).  It might even work on
> Windows, although this has not been tested. The shared memory file's
> name folows the template "shm-<object>-<SHA1>" where <SHA1> is the

s/folows/follows/

And: now that it is no longer shared memory, should we not do away with
the "shm-" prefix?

Show 5 quoted lines

> trailing SHA-1 of the index file. <object> is "index" for cached index
> files (and might later be "name-hash" for name-hash cache). If such
> shared memory exists, it contains the same index content as on
> disk. The content is already validated by the daemon and git won't
> validate it again (except comparing the trailing SHA-1s).

Excellent. I was worrying about the penalty of validating.

> We keep this daemon's logic as thin as possible. The "brain" stays in
> git. So the daemon can read and validate stuff, but that's all it's
> allowed to do. It does not add/create new information. It doesn't even
> accept direct updates from git.

I like this design. For now. Later, I really think we should add the ability to update the index via the index-helper. I am thinking about a method similar to watchman where a separate process (that may use the inotify syscall on Linux, or tap into the NTFS journal on Windows) tells the index-helper specifically which paths to look at, so that nobody ever has to look at any files that were not modified at all.

> +	if (*new_mmap == MAP_FAILED) {
> +		*new_mmap = NULL;
> +		goto done;

Shouldn't we provide some sort of error message here?

> +	}
> +	madvise(new_mmap, size, MADV_WILLNEED);

I guess I'll need to add support for that to compat/win32mmap.c (most likely using PrefetchVirtualMemory() when available, i.e. Windows >= 8, see https://msdn.microsoft.com/en-us/library/windows/desktop/hh780543.aspx :-))

Likewise, I think it will be easy to use bidirectional named pipes on Windows to accommodate for the lack of Unix domain sockets...

Ciao, Dscho