refactor(watch): create single watcher for whole process

[magurotuna]

This is a follow-up PR to #7612
Fixes small part of #7465, so that we won't create file watcher every time on file updating.

When running cargo run -- run --watch --unstable foo.js and update the content of foo.js, it seems like I've got results as expected.
The integration test run_watch fails locally right now though. I'm looking into it.

cc @bartlomieju

[magurotuna]

https://github.com/denoland/deno/pull/8083/checks?check_run_id=1295812207#step:15:1313
hmm looks like run_watch fails only on Linux (my machine is also Linux).

It might be a problem with notify crate. We currently use notify v5.0.0-pre.3, but I want to try v.4.0.15 instead, which is the latest stable version. Moreover, v4.0.15 has the functionality of handling "debounced" events, so we could remove our debouncing logic from our codebase.
https://docs.rs/notify/4.0.15/notify/trait.Watcher.html

[bartlomieju]

@magurotuna I'd be cautious about downgrading notify - we also use it in Deno.fsEvents and it might be pretty hard to replace it. Could you elaborate what's the problem you're experiencing?

[magurotuna]

@bartlomieju

Could you elaborate what's the problem you're experiencing?

Oh, I didn't notice notify is also used for Deno.fsEvents. I'll try to find a solution without downgrading.

[magurotuna]

run_watch is writing a file by calling std::fs::write. On Linux, when calling std::fs::write, it seems like the following events are what we get via notify:

[cli/file_watcher.rs:156] &res = Ok(
    Event {
        kind: Modify(
            Data(
                Any,
            ),
        ),
        paths: [
            "/tmp/deno-watcher/foo.js",
        ],
        attr:tracker: None,
        attr:flag: None,
        attr:info: None,
        attr:source: None,
    },
)
[cli/file_watcher.rs:156] &res = Ok(
    Event {
        kind: Access(
            Close(
                Write,
            ),
        ),
        paths: [
            "/tmp/deno-watcher/foo.js",
        ],
        attr:tracker: None,
        attr:flag: None,
        attr:info: None,
        attr:source: None,
    },
)

On the other hand, if I hit :w to save the file in vim, events change to:

[cli/file_watcher.rs:146] &res = Ok(
    Event {
        kind: Modify(
            Data(
                Any,
            ),
        ),
        paths: [
            "/tmp/deno-watcher/foo.js",
        ],
        attr:tracker: None,
        attr:flag: None,
        attr:info: None,
        attr:source: None,
    },
)
[cli/file_watcher.rs:146] &res = Ok(
    Event {
        kind: Access(
            Close(
                Write,
            ),
        ),
        paths: [
            "/tmp/deno-watcher/foo.js",
        ],
        attr:tracker: None,
        attr:flag: None,
        attr:info: None,
        attr:source: None,
    },
)
[cli/file_watcher.rs:146] &res = Ok(
    Event {
        kind: Modify(
            Metadata(
                Any,
            ),
        ),
        paths: [
            "/tmp/deno-watcher/foo.js",
        ],
        attr:tracker: None,
        attr:flag: None,
        attr:info: None,
        attr:source: None,
    },
)

In the current implementation, we treat EventKind::Create | EventKind::Modify | EventKind::Remove as the triggers for reloading, but in the case of std::fs::write, the event kind we get last is EventKind::Access. This is why run_watch test fails.
I'll try to elaborate some solution.

[magurotuna]

For now, I have added 10ms sleep to avoid a hot loop, referring to #7612 (comment)
On my machine, CPU usage of Deno's process has reduced to ~3.0% after adding a sleep. (Without sleep, it was 100%)
This looks fine to me 😃

[bartlomieju]

Discussed offline with @magurotuna and @caspervonb. Given bugs in watcher implementation we decided to postpone this PR and rewrite watcher properly.

[caspervonb]

LGTM, doesn't seem to spike above 1%

[magurotuna]

The remaining issue I'm digging into is that we get multiple consecutive "Watcher File changed detected! Restarting!" message. @bartlomieju gave me information about it.

terminal

hello world
Watcher Process terminated! Restarting on file change...
Watcher File change detected! Restarting!
Watcher File change detected! Restarting!
a
b
c
hello world
Watcher Process terminated! Restarting on file change...
Watcher File change detected! Restarting!
Watcher File change detected! Restarting!
a
b
c
hello world
Watcher Process terminated! Restarting on file change...
Watcher File change detected! Restarting!
a
b
c
hello world
Watcher Process terminated! Restarting on file change...
Watcher File change detected! Restarting!
Watcher File change detected! Restarting!
Watcher File change detected! Restarting!
a
b
c
hello world
Watcher Process terminated! Restarting on file change...

How to reproduce

1. create main.js with the following content:

import { a } from "./test2/a.js";
import { b } from "./test2/b.js";
import { c } from "./test2/c.js";

console.log(a);
console.log(b);
console.log(c);
console.log("hello world");

2. run deno run --watch --unstable main.js
3. run mv ./test2 ./test1
4. replace test2 with test1 in main.js and save it

[magurotuna]

I gave it a try to reproduce it. On my main machine, where Manjaro 20.1.2 is running, the output I saw looks like:

Well, actually the watcher didn't anything at all when I ran mv ./test2 ./test1, while it detected file change only when replacing test2 with test1 in main.js and saving it.

To see more detail, I inserted dbg! in the watcher callback so I can see what kind of event the watcher detects. But in fact, at the time when I ran mv ./test2 ./test1, the watcher didn't get any kind of event.

I also tried it on macOS. The result is almost identical, except for the events' kind which I think doesn't matter.

[magurotuna]

Talked with @bartlomieju about the above issue, and it turned out that the watcher actually works as we expect. So this PR is ready to be merged.
Please merge it when it's convenient.

[bartlomieju]

LGTM, thanks @magurotuna, @bartossh, @caspervonb, this is a great improvement