watch: optimization to help avoid inotify nodes for large file trees (docker#5769)

fixes tilt-dev/tilt#5764

Author: nicks

pkg/watch/watcher_naive.go

@@ -26,6 +26,10 @@ type naiveNotify struct {
 	// Paths that we're watching that should be passed up to the caller.
 	// Note that we may have to watch ancestors of these paths
 	// in order to fulfill the API promise.
+	//
+	// We often need to check if paths are a child of a path in
+	// the notify list. It might be better to store this in a tree
+	// structure, so we can filter the list quickly.
 	notifyList map[string]bool
 
 	ignore PathMatcher
@@ -226,7 +230,7 @@ func (d *naiveNotify) shouldNotify(path string) bool {
 		}
 		return true
 	}
-	// TODO(dmiller): maybe use a prefix tree here?
+
 	for root := range d.notifyList {
 		if ospath.IsChild(root, path) {
 			return true
@@ -245,7 +249,29 @@ func (d *naiveNotify) shouldSkipDir(path string) (bool, error) {
 	if err != nil {
 		return false, errors.Wrap(err, "shouldSkipDir")
 	}
-	return skip, nil
+
+	if skip {
+		return true, nil
+	}
+
+	// Suppose we're watching
+	// /src/.tiltignore
+	// but the .tiltignore file doesn't exist.
+	//
+	// Our watcher will create an inotify watch on /src/.
+	//
+	// But then we want to make sure we don't recurse from /src/ down to /src/node_modules.
+	//
+	// To handle this case, we only want to traverse dirs that are:
+	// - A child of a directory that's in our notify list, or
+	// - A parent of a directory that's in our notify list
+	//   (i.e., to cover the "path doesn't exist" case).
+	for root := range d.notifyList {
+		if ospath.IsChild(root, path) || ospath.IsChild(path, root) {
+			return false, nil
+		}
+	}
+	return true, nil
 }
 
 func (d *naiveNotify) add(path string) error {

pkg/watch/watcher_naive_test.go

@@ -7,10 +7,13 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestDontWatchEachFile(t *testing.T) {
@@ -96,20 +99,48 @@ func TestDontWatchEachFile(t *testing.T) {
 	}
 	f.events = nil
 
+	n, err := inotifyNodes()
+	require.NoError(t, err)
+	if n > 10 {
+		t.Fatalf("watching more than 10 files: %d", n)
+	}
+}
+
+func inotifyNodes() (int, error) {
 	pid := os.Getpid()
 
 	output, err := exec.Command("bash", "-c", fmt.Sprintf(
 		"find /proc/%d/fd -lname anon_inode:inotify -printf '%%hinfo/%%f\n' | xargs cat | grep -c '^inotify'", pid)).Output()
 	if err != nil {
-		t.Fatalf("error running command to determine number of watched files: %v", err)
+		return 0, fmt.Errorf("error running command to determine number of watched files: %v", err)
 	}
 
 	n, err := strconv.Atoi(strings.TrimSpace(string(output)))
 	if err != nil {
-		t.Fatalf("couldn't parse number of watched files: %v", err)
+		return 0, fmt.Errorf("couldn't parse number of watched files: %v", err)
 	}
+	return n, nil
+}
 
-	if n > 10 {
-		t.Fatalf("watching more than 10 files: %d", n)
+func TestDontRecurseWhenWatchingParentsOfNonExistentFiles(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("This test uses linux-specific inotify checks")
+	}
+
+	f := newNotifyFixture(t)
+
+	watched := f.TempDir("watched")
+	f.watch(filepath.Join(watched, ".tiltignore"))
+
+	excludedDir := f.JoinPath(watched, "excluded")
+	for i := 0; i < 10; i++ {
+		f.WriteFile(f.JoinPath(excludedDir, fmt.Sprintf("%d", i), "data.txt"), "initial data")
+	}
+	f.fsync()
+
+	n, err := inotifyNodes()
+	require.NoError(t, err)
+	if n > 5 {
+		t.Fatalf("watching more than 5 files: %d", n)
 	}
 }