Fix multiple reloads in one change

[012e]

Simply fix multiple reloads in one change

[matthewmueller]

Hi @012e thanks for your effort, but I don't think this solution is going to work.

• It might work on the first reload, but not subsequent reloads
• If you subtract 1ms every reload, you're back at where it is right now

I'd recommend trying the solution I suggested earlier.

By the way, I'm a bit confused why the Ubuntu tests are passing the watcher, but the watcher isn't working for your machine. Do you have any idea why that might be?

[012e]

This code only initialize the startTime variable once, minus by 1ms to ensure it won't get ignore by time.Since(startTime).Milliseconds() >= 1.

startTime := time.Now().Add(-1 * time.Millisecond)

Each time an event is triggered startTtime will be reset.

startTime = time.Now()

For example, if 2 events are triggered in one change, 0ms delay between each. Then the first event runs normally. On the second event, time between (4) to (2) to (3) is 0ms and the event is ignored.

The code will fail to keep up with if you make 2 or more file changes (save) in less than 1ms. But I don't think any human being can do this. Too see the delay, you can try

case evt := <-watcher.Events:
        // Add here
    fmt.Printf("Time between previous event: %s\n", time.Since(startTime).String())
    switch op := evt.Op; {

I tested it on my local machine, it works properly, there's no extra reloads. I did try Github Actions on my fork before make a PR. It works.

Now I am also confused 😕.

[012e]

By the way, I'm a bit confused why the Ubuntu tests are passing the watcher, but the watcher isn't working for your machine. Do you have any idea why that might be?

1. Probably because it changes the file one time, and getEvent only expect at least a response to pass the test. So if there are more, the test still doesn't fail. https://github.com/livebud/bud/blob/ee97bd4d1baceaa45feaaa612f1619985c29ea1f/package/watcher/watcher_test.go#L35-L41
2. Is it something related to system hardware or drivers + OS?

[matthewmueller]

Hmm, okay let me pull this done and test it locally! Cool diagram btw 😄

[012e]

I think this will work 😀

[matthewmueller]

Hey @012e, just pulled it down. It seems to be working.

With this PR, I'm still a bit hesitant: if a single new file change occurs within 1 millisecond of the last file change, the last file change is lost, leading bud/ build to be older than what's in your filesystem. This is hypothetical though since I'm not able to reproduce this behavior.

Can you remind me of the problem this solves for you? From the previous PR it was to avoid multiple triggers, is that still the problem? The fix may be worth the hypothetical issue above.

[012e]

My issue:

It reloads multiple times because of multiple write signals. Normally, when I make a single file change while bud run:

| Ready on http://127.0.0.1:3000
| Ready on http://127.0.0.1:3000
| Ready on http://127.0.0.1:3000
| Ready on http://127.0.0.1:3000

• If I add this line here (fix-multiple-reloads branch)

case evt := <-watcher.Events:
        // Add here
         fmt.Printf("Current event: %s, time between previous event: %s\n", evt, time.Since(startTime).String())
    switch op := evt.Op; {

• Each time I make a single file save:

Current event: "controller/controller.go": WRITE, time between previous event: 50.407399441s
| Ready on http://127.0.0.1:3000
Current event: "controller/controller.go": WRITE, time between previous event: 2.25µs
Current event: "controller/controller.go": WRITE, time between previous event: 27.773µs
Current event: "controller/controller.go": WRITE, time between previous event: 33.173µs

bud run

if a single new file change occurs within 1 millisecond of the last file change, the last file change is lost

OK, because the write handler wasn't triggered on the second change (event triggered within 1ms cool down period)

leading bud/ build to be older than what's in your filesystem

Uh, since the watcher didn't run both write event (only the first write), only a single re-compile is made (for first write). This is the flaw of using a cool down, not much way to prevent.

[matthewmueller]

Just tested again, you're not seeing 2 events per save on your linux? I created a test script:

scripts/test-watcher/main.go

package main

import (
    "context"
    "fmt"
    "log"
    "os"

    "github.com/livebud/bud/internal/current"

    "github.com/livebud/bud/internal/sig"
    "github.com/livebud/bud/package/watcher"
)

func run(ctx context.Context) error {
    dirname, err := current.Directory()
    if err != nil {
        return err
    }
    ctx = sig.Trap(ctx, os.Interrupt)
    return watcher.Watch(ctx, dirname, func(path string) error {
        fmt.Println("-> triggered", path)
        return nil
    })
}

func main() {
    ctx := context.Background()
    if err := run(ctx); err != nil {
        log.Fatal(err)
    }
}

scripts/test-watcher/a.txt

aa

Then using the Ubuntu image in the contributing Readme

docker build -t bud:latest contributing
docker run -it --rm -v $(PWD):/bud bud /bin/bash

Results:

docker run -it --rm -v $(PWD):/bud bud /bin/bash
root@5d9c4cac4118:/bud# go run scripts/test-watcher/main.go 
-> triggered /bud/scripts/test-watcher/a.txt
-> triggered /bud/scripts/test-watcher/a.txt

Whenever I make a change, it triggers twice. This is still less than the 4 times I'm seeing per change without your adjustment.