Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watcher backends: merge 'add' and 'change' events into 'touch' #1417

Closed
wants to merge 1 commit into from

Conversation

robhogan
Copy link
Contributor

Summary:
Refactor watcher backends so that they're not required to disambiguate "add" file events from "change" file events, instead emitting ambiguous "touch" events.

This distinction isn't typically available from the host OS (fsevents, ReadDirectoryChangesW, inotify), so for the backends to supply it, they must track all files to know whether they existed prior to the OS event. Watchman does this by design anyway, and typically only once per session, but both our FSEventsWatcher and NodeWatcher currently crawl the whole directory tree and keep an in-memory list of files, and must do so on each Metro start.

We don't need this information from the watchers anyway, because we can already infer new vs modified according to whether the file is present in TreeFS (ie, from the crawl, or subsequent event). Therefore we don't need to reduce the granularity of metro-file-map's public events, only the internal ones.

By simplifying this, we can follow up by taking a huge burden off the watcher backends.

Changelog: Internal

Differential Revision: D67579233

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 22, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67579233

robhogan added a commit that referenced this pull request Dec 25, 2024
Summary:
Pull Request resolved: #1417

Refactor watcher backends so that they're not required to disambiguate "add" file events from "change" file events, instead emitting ambiguous "touch" events.

This distinction isn't typically available from the host OS (`fsevents`, `ReadDirectoryChangesW`, `inotify`), so for the backends to supply it, they must track all files to know whether they existed prior to the OS event. Watchman does this by design anyway, and typically only once per session, but both our `FSEventsWatcher` and `NodeWatcher` currently crawl the whole directory tree and keep an in-memory list of files, and must do so on each Metro start.

We don't need this information from the watchers anyway, because we can already infer new vs modified according to whether the file is present in `TreeFS` (ie, from the crawl, or subsequent event). Therefore we *don't* need to reduce the granularity of `metro-file-map`'s *public* events, only the internal ones.

By simplifying this, we can follow up by taking a huge burden off the watcher backends.

Changelog: Internal

Differential Revision: D67579233
Summary:

Refactor watcher backends so that they're not required to disambiguate "add" file events from "change" file events, instead emitting ambiguous "touch" events.

This distinction isn't typically available from the host OS (`fsevents`, `ReadDirectoryChangesW`, `inotify`), so for the backends to supply it, they must track all files to know whether they existed prior to the OS event. Watchman does this by design anyway, and typically only once per session, but both our `FSEventsWatcher` and `NodeWatcher` currently crawl the whole directory tree and keep an in-memory list of files, and must do so on each Metro start.

We don't need this information from the watchers anyway, because we can already infer new vs modified according to whether the file is present in `TreeFS` (ie, from the crawl, or subsequent event). Therefore we *don't* need to reduce the granularity of `metro-file-map`'s *public* events, only the internal ones.

By simplifying this, we can follow up by taking a huge burden off the watcher backends.

Changelog: Internal

Reviewed By: blakef

Differential Revision: D67579233
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67579233

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 4d4300a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants