better io tracking#1666
Open
robhogan wants to merge 12 commits into
Open
Conversation
Summary: Adds optional mutation tracking to FileSystem's mutation methods via a FileSystemListener parameter, and add an implementation that aggregates *net* changes into a batch. The three file change maps are mutually exclusive, as are the two directory change sets. Differential Revision: D92006310
…rocessing Summary: ## This stack Directory creation/deletion events are essential to efficient incremental resolution, because the resolver uses non-existence of a directory (like `node_modules`) to skip dozens of file existence checks under that directory. However, directory addition/deletion is reported inconsistently at the operating system level, and any low-level tracking must be implemented separately for each watcher. Instead, we'll reorganise things so that file events are passed to `FileSystem` (`TreeFS`), and *it* reports what changes are actually made to the in-memory representation, including which directories are added/removed. This turns out to be cleaner for file events too, and unlocks fixes like better handling of directory moves. ## This diff Uses the new `FileSystemChangeAggregator` to track files added/modified/and removed during crawl (ie, between one startup and the next, with or without a cache), which cleans up some ad-hoc Differential Revision: D92047480
…mmediately before each emit Differential Revision: D92008990
…ectly
Summary:
## This stack
Directory creation/deletion events are essential to efficient incremental resolution, because the resolver uses non-existence of a directory (like `node_modules`) to skip dozens of file existence checks under that directory.
However, directory addition/deletion is reported inconsistently at the operating system level, and any low-level tracking must be implemented separately for each watcher. Instead, we'll reorganise things so that file events are passed to `FileSystem` (`TreeFS`), and *it* reports what changes are actually made to the in-memory representation, including which directories are added/removed. This turns out to be cleaner for file events too, and unlocks fixes like better handling of directory moves.
## This diff
Consolidates the plugin update API from three separate methods (`bulkUpdate`, `onNewOrModifiedFile`, `onRemovedFile`) into a single unified `onChanged(changes: ReadonlyFileSystemChanges<T>)` method.
Previously, plugins implemented three different change handlers:
- `bulkUpdate(delta)` for batch processing with `{addedOrModified, removed}` iterables
- `onNewOrModifiedFile(path, data)` for individual file additions/modifications
- `onRemovedFile(path, data)` for individual file removals
This diff replaces all three with a single `onChanged` method that receives a `ReadonlyFileSystemChanges` object containing categorized change iterables: `addedFiles`, `modifiedFiles`, `removedFiles`, `addedDirectories`, and `removedDirectories`.
Key benefits:
1. **Simpler plugin interface** - One method instead of three reduces boilerplate and implementation burden
2. **Richer change semantics** - Distinguishes added vs modified files (previously conflated in `addedOrModified`)
3. **Directory tracking** - Exposes directory add/remove events, enabling future features like incremental resolution invalidation based on directory existence
Also moves `DependencyPlugin-test.js` and `dependencyExtractor.js` to colocate test fixtures with plugin tests under `plugins/dependencies/__tests__/`.
Differential Revision: D93984442
…ngeAggregator and reduce duplication Summary: ## This stack Directory creation/deletion events are essential to efficient incremental resolution, because the resolver uses non-existence of a directory (like `node_modules`) to skip dozens of file existence checks under that directory. However, directory addition/deletion is reported inconsistently at the operating system level, and any low-level tracking must be implemented separately for each watcher. Instead, we'll reorganise things so that file events are passed to `FileSystem` (`TreeFS`), and *it* reports what changes are actually made to the in-memory representation, including which directories are added/removed. This turns out to be cleaner for file events too, and unlocks fixes like better handling of directory moves. ## This diff A bit of misc refactoring that decouples things from the `eventsQueue` we're about to remove. Differential Revision: D94076918
…ventsQueue, update tests to use new API Differential Revision: D94077032
…eEvent API
Summary:
Refactors DeltaCalculator tests to use the new `ChangeEvent` API with structured `changes` and `rootDir` fields, replacing the deprecated `eventsQueue` format.
**Key changes:**
1. **New shared test utility** (`test-utils.js`):
- Adds `createEmitChange` factory function that creates an `emitChange` helper for emitting file system change events
- Supports a concise API: `emitChange({addedFiles: ['foo'], modifiedFiles: ['bar']})`
- File entries can be strings or tuples with metadata: `['link', {isSymlink: true}]`
- Handles platform-specific path separators for Windows compatibility
- Builds the full `ChangeEvent` structure with `rootDir`, `changes` (containing `addedFiles`, `modifiedFiles`, `removedFiles`, `addedDirectories`, `removedDirectories`), and `logger`
2. **DeltaCalculator-test.js**:
- Imports and uses `createEmitChange` from shared utilities
- Replaces verbose `fileWatcher.emit('change', createChangeEvent(...))` calls with concise `emitChange({...})` calls
- Uses canonical paths (relative to `rootDir`) instead of absolute paths in test events
- Fixes unhandled promise rejection by using `mockRejectedValue` instead of `mockReturnValue(Promise.reject(...))`
3. **DeltaCalculator-context-test.js**:
- Imports and uses `createEmitChange` from shared utilities
- Removes local `createChangeEvent` helper in favor of shared `emitChange`
- Updates all test call sites to use the new concise API
Differential Revision: D95057371
…Queue Differential Revision: D94080398
…entalResolution Differential Revision: D95796617
Summary: Refactors the `hierarchicalLookup` API in TreeFS/FileSystem to support incremental resolution invalidation tracking: - Adds `InvalidationData` type with `existence`, `modification`, and `haste` sets, using canonical (root-relative) paths for efficiency and compactness. - Consolidates the `invalidatedBy` parameter from the old `?Set<string>` to `?InvalidationData`, which separates existence dependencies (path added/removed) from modification dependencies (symlink target changed). - Adds `collectLinkNormalPaths` option to `#lookupByNormalPath` to collect canonical symlink paths separately from absolute paths. - Writes canonical paths instead of absolute paths, which are cheaper to store and compare. - Updates all callers, TypeScript declarations, and tests. Differential Revision: D95796618
…atedBy` Differential Revision: D95796615
Differential Revision: D95796616
meta-cla
Bot
added
the
CLA Signed
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Differential Revision: D95796616