diff --git a/CHANGELOG.md b/CHANGELOG.md index d011a492464..cd0a86752a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## Unreleased + +* Improve watch mode accuracy ([#1113](https://github.com/evanw/esbuild/issues/1113)) + + Watch mode is enabled by `--watch` and causes esbuild to become a long-running process that automatically rebuilds output files when input files are changed. It's implemented by recording all calls to esbuild's internal file system interface and then invalidating the build whenever these calls would return different values. For example, a call to esbuild's internal `ReadFile()` function is considered to be different if either the presence of the file has changed (e.g. the file didn't exist before but now exists) or the presence of the file stayed the same but the content of the file has changed. + + Previously esbuild's watch mode operated at the `ReadFile()` and `ReadDirectory()` level. When esbuild checked whether a directory entry existed or not (e.g. whether a directory contains a `node_modules` subdirectory or a `package.json` file), it called `ReadDirectory()` which then caused the build to depend on that directory's set of entries. This meant the build would be invalidated even if a new unrelated entry was added or removed, since that still changes the set of entries. This is problematic when using esbuild in environments that constantly create and destroy temporary directory entries in your project directory. In that case, esbuild's watch mode would constantly rebuild as the directory was constantly considered to be dirty. + + With this release, watch mode now operates at the `ReadFile()` and `ReadDirectory().Get()` level. So when esbuild checks whether a directory entry exists or not, the build should now only depend on the presence status for that one directory entry. This should avoid unnecessary rebuilds due to unrelated directory entries being added or removed. The log messages generated using `--watch` will now also mention the specific directory entry whose presence status was changed if a build is invalidated for this reason. + + Note that this optimization does not apply to plugins using the `watchDirs` return value because those paths are only specified at the directory level and do not describe individual directory entries. You can use `watchFiles` or `watchDirs` on the individual entries inside the directory to get a similar effect instead. + ## 0.13.4 * Fix permission issues with the install script ([#1642](https://github.com/evanw/esbuild/issues/1642)) diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index 645502b7850..8333be76096 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -697,7 +697,9 @@ func runOnResolvePlugins( fsCache.ReadFile(fs, file) } for _, dir := range result.AbsWatchDirs { - fs.ReadDirectory(dir) + if entries, err, _ := fs.ReadDirectory(dir); err == nil { + entries.SortedKeys() + } } // Stop now if there was an error @@ -812,7 +814,9 @@ func runOnLoadPlugins( fsCache.ReadFile(fs, file) } for _, dir := range result.AbsWatchDirs { - fs.ReadDirectory(dir) + if entries, err, _ := fs.ReadDirectory(dir); err == nil { + entries.SortedKeys() + } } // Stop now if there was an error diff --git a/internal/fs/fs.go b/internal/fs/fs.go index e85f75614bf..83055eec87c 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -3,6 +3,7 @@ package fs import ( "errors" "os" + "sort" "strings" "sync" "syscall" @@ -44,17 +45,37 @@ func (e *Entry) Symlink(fs FS) string { return e.symlink } -type DirEntries struct { - dir string - data map[string]*Entry +type accessedEntries struct { + mutex sync.Mutex + wasPresent map[string]bool + + // If this is nil, "SortedKeys()" was not accessed. This means we should + // check for whether this directory has changed or not by seeing if any of + // the entries in the "wasPresent" map have changed in "present or not" + // status, since the only access was to individual entries via "Get()". + // + // If this is non-nil, "SortedKeys()" was accessed. This means we should + // check for whether this directory has changed or not by checking the + // "allEntries" array for equality with the existing entries list, since the + // code asked for all entries and may have used the presence or absence of + // entries in that list. + // + // The goal of having these two checks is to be as narrow as possible to + // avoid unnecessary rebuilds. If only "Get()" is called on a few entries, + // then we won't invalidate the build if random unrelated entries are added + // or removed. But if "SortedKeys()" is called, we need to invalidate the + // build if anything about the set of entries in this directory is changed. + allEntries []string } -func MakeEmptyDirEntries(dir string) DirEntries { - return DirEntries{dir, make(map[string]*Entry)} +type DirEntries struct { + dir string + data map[string]*Entry + accessedEntries *accessedEntries } -func (entries DirEntries) Len() int { - return len(entries.data) +func MakeEmptyDirEntries(dir string) DirEntries { + return DirEntries{dir, make(map[string]*Entry), nil} } type DifferentCase struct { @@ -65,7 +86,17 @@ type DifferentCase struct { func (entries DirEntries) Get(query string) (*Entry, *DifferentCase) { if entries.data != nil { - if entry := entries.data[strings.ToLower(query)]; entry != nil { + key := strings.ToLower(query) + entry := entries.data[key] + + // Track whether this specific entry was present or absent for watch mode + if accessed := entries.accessedEntries; accessed != nil { + accessed.mutex.Lock() + accessed.wasPresent[key] = entry != nil + accessed.mutex.Unlock() + } + + if entry != nil { if entry.base != query { return entry, &DifferentCase{ Dir: entries.dir, @@ -76,16 +107,28 @@ func (entries DirEntries) Get(query string) (*Entry, *DifferentCase) { return entry, nil } } + return nil, nil } -func (entries DirEntries) UnorderedKeys() (keys []string) { +func (entries DirEntries) SortedKeys() (keys []string) { if entries.data != nil { keys = make([]string, 0, len(entries.data)) for _, entry := range entries.data { keys = append(keys, entry.base) } + sort.Strings(keys) + + // Track the exact set of all entries for watch mode + if entries.accessedEntries != nil { + entries.accessedEntries.mutex.Lock() + entries.accessedEntries.allEntries = keys + entries.accessedEntries.mutex.Unlock() + } + + return keys } + return } @@ -152,8 +195,11 @@ type FS interface { } type WatchData struct { - // These functions return true if the file system entry has been modified - Paths map[string]func() bool + // These functions return a non-empty path as a string if the file system + // entry has been modified. For files, the returned path is the same as the + // file path. For directories, the returned path is either the directory + // itself or a file in the directory that was changed. + Paths map[string]func() string } type ModKey struct { diff --git a/internal/fs/fs_mock.go b/internal/fs/fs_mock.go index 1bfffd8fc18..22b77b7a1cc 100644 --- a/internal/fs/fs_mock.go +++ b/internal/fs/fs_mock.go @@ -29,7 +29,7 @@ func MockFS(input map[string]string) FS { kDir := path.Dir(k) dir, ok := dirs[kDir] if !ok { - dir = DirEntries{kDir, make(map[string]*Entry)} + dir = DirEntries{kDir, make(map[string]*Entry), nil} dirs[kDir] = dir } if kDir == k { diff --git a/internal/fs/fs_real.go b/internal/fs/fs_real.go index d4831dfe30b..c1d0911700e 100644 --- a/internal/fs/fs_real.go +++ b/internal/fs/fs_real.go @@ -38,20 +38,20 @@ type entriesOrErr struct { type watchState uint8 const ( - stateNone watchState = iota - stateDirHasEntries // Compare "dirEntries" - stateDirMissing // Compare directory presence - stateFileHasModKey // Compare "modKey" - stateFileNeedModKey // Need to transition to "stateFileHasModKey" or "stateFileUnusableModKey" before "WatchData()" returns - stateFileMissing // Compare file presence - stateFileUnusableModKey // Compare "fileContents" + stateNone watchState = iota + stateDirHasAccessedEntries // Compare "accessedEntries" + stateDirMissing // Compare directory presence + stateFileHasModKey // Compare "modKey" + stateFileNeedModKey // Need to transition to "stateFileHasModKey" or "stateFileUnusableModKey" before "WatchData()" returns + stateFileMissing // Compare file presence + stateFileUnusableModKey // Compare "fileContents" ) type privateWatchData struct { - dirEntries []string - fileContents string - modKey ModKey - state watchState + accessedEntries *accessedEntries + fileContents string + modKey ModKey + state watchState } type RealFSOptions struct { @@ -133,7 +133,11 @@ func (fs *realFS) ReadDirectory(dir string) (entries DirEntries, canonicalError // Cache miss: read the directory entries names, canonicalError, originalError := fs.readdir(dir) - entries = DirEntries{dir, make(map[string]*Entry)} + entries = DirEntries{dir, make(map[string]*Entry), nil} + + if fs.watchData != nil { + entries.accessedEntries = &accessedEntries{wasPresent: make(map[string]bool)} + } // Unwrap to get the underlying error if pathErr, ok := canonicalError.(*os.PathError); ok { @@ -157,14 +161,13 @@ func (fs *realFS) ReadDirectory(dir string) (entries DirEntries, canonicalError if fs.watchData != nil { defer fs.watchMutex.Unlock() fs.watchMutex.Lock() - state := stateDirHasEntries + state := stateDirHasAccessedEntries if canonicalError != nil { state = stateDirMissing } - sort.Strings(names) fs.watchData[dir] = privateWatchData{ - dirEntries: names, - state: state, + accessedEntries: entries.accessedEntries, + state: state, } } @@ -432,7 +435,7 @@ func (fs *realFS) kind(dir string, base string) (symlink string, kind EntryKind) } func (fs *realFS) WatchData() WatchData { - paths := make(map[string]func() bool) + paths := make(map[string]func() string) for path, data := range fs.watchData { // Each closure below needs its own copy of these loop variables @@ -454,42 +457,70 @@ func (fs *realFS) WatchData() WatchData { switch data.state { case stateDirMissing: - paths[path] = func() bool { + paths[path] = func() string { info, err := os.Stat(path) - return err == nil && info.IsDir() + if err == nil && info.IsDir() { + return path + } + return "" } - case stateDirHasEntries: - paths[path] = func() bool { + case stateDirHasAccessedEntries: + paths[path] = func() string { names, err, _ := fs.readdir(path) - if err != nil || len(names) != len(data.dirEntries) { - return true + if err != nil { + return path } - sort.Strings(names) - for i, s := range names { - if s != data.dirEntries[i] { - return true + data.accessedEntries.mutex.Lock() + defer data.accessedEntries.mutex.Unlock() + if allEntries := data.accessedEntries.allEntries; allEntries != nil { + // Check all entries + if len(names) != len(allEntries) { + return path + } + sort.Strings(names) + for i, s := range names { + if s != allEntries[i] { + return path + } + } + } else { + // Check individual entries + isPresent := make(map[string]bool, len(names)) + for _, name := range names { + isPresent[strings.ToLower(name)] = true + } + for name, wasPresent := range data.accessedEntries.wasPresent { + if wasPresent != isPresent[name] { + return fs.Join(path, name) + } } } - return false + return "" } case stateFileMissing: - paths[path] = func() bool { - info, err := os.Stat(path) - return err == nil && !info.IsDir() + paths[path] = func() string { + if info, err := os.Stat(path); err == nil && !info.IsDir() { + return path + } + return "" } case stateFileHasModKey: - paths[path] = func() bool { - key, err := modKey(path) - return err != nil || key != data.modKey + paths[path] = func() string { + if key, err := modKey(path); err != nil || key != data.modKey { + return path + } + return "" } case stateFileUnusableModKey: - paths[path] = func() bool { - buffer, err := ioutil.ReadFile(path) - return err != nil || string(buffer) != data.fileContents + paths[path] = func() string { + if buffer, err := ioutil.ReadFile(path); err != nil || string(buffer) != data.fileContents { + return path + } + return "" } } } diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index d8f1118e3d6..a1e9e562b08 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -796,7 +796,7 @@ func (r resolverQuery) dirInfoCached(path string) *dirInfo { if cached == nil { r.debugLogs.addNote(fmt.Sprintf("Failed to read directory %q", path)) } else { - count := cached.entries.Len() + count := len(cached.entries.SortedKeys()) entries := "entries" if count == 1 { entries = "entry" diff --git a/pkg/api/api_impl.go b/pkg/api/api_impl.go index 89629d5ad44..ecf993492dd 100644 --- a/pkg/api/api_impl.go +++ b/pkg/api/api_impl.go @@ -1212,11 +1212,11 @@ func (w *watcher) tryToFindDirtyPath() string { // Always check all recent items every iteration for i, path := range w.recentItems { - if w.data.Paths[path]() { + if dirtyPath := w.data.Paths[path](); dirtyPath != "" { // Move this path to the back of the list (i.e. the "most recent" position) copy(w.recentItems[i:], w.recentItems[i+1:]) w.recentItems[len(w.recentItems)-1] = path - return path + return dirtyPath } } @@ -1230,7 +1230,7 @@ func (w *watcher) tryToFindDirtyPath() string { // Check if any of the entries in this iteration have been modified for _, path := range toCheck { - if w.data.Paths[path]() { + if dirtyPath := w.data.Paths[path](); dirtyPath != "" { // Mark this item as recent by adding it to the back of the list w.recentItems = append(w.recentItems, path) if len(w.recentItems) > maxRecentItemCount { @@ -1238,7 +1238,7 @@ func (w *watcher) tryToFindDirtyPath() string { copy(w.recentItems, w.recentItems[1:]) w.recentItems = w.recentItems[:maxRecentItemCount] } - return path + return dirtyPath } } return "" diff --git a/pkg/api/serve_other.go b/pkg/api/serve_other.go index f7f08f8dba2..a999a02de80 100644 --- a/pkg/api/serve_other.go +++ b/pkg/api/serve_other.go @@ -193,7 +193,7 @@ func (h *apiHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) { if h.servedir != "" && kind != fs.FileEntry { if entries, err, _ := h.fs.ReadDirectory(h.fs.Join(h.servedir, queryPath)); err == nil { kind = fs.DirEntry - for _, name := range entries.UnorderedKeys() { + for _, name := range entries.SortedKeys() { entry, _ := entries.Get(name) switch entry.Kind(h.fs) { case fs.DirEntry: