timholy
Skip sleep(0.01) for non-change triggered Revise.revise()
Revise.revise() is typically used in one of two ways:
- Called just before code is executed (non-change triggered)
- Called when files are modified (change triggered)
Revise.revise() does a sleep(0.01) in either case
https://github.com/timholy/Revise.jl/blob/13a5eb7986ee1239ff938a92043c13fee04579cc/src/packagedef.jl#L751
This is clearly a bit of a hack in the first place, but it does make sense that it might help for change triggered Revise. However, if Revise.revise() is being run in a non-change triggered scenario, there is no reason to think 10ms from now is any better than now. Some non-change triggered uses of Revise, such as the REPL hook, mitigate this by guarding Revise.revise() with isempty(revision_queue), however even if there are revisions, is there any reason to wait 10ms extra in this scenario? It's not so long, but latency adds up.
The simplest solution would be to add a flag e.g. Revise.revise(...; ... skip_sleep=false), and change all non-change triggered usages to set it to true.
Can you give me more information about the non-change triggered uses? I pretty much only use it in the change-triggered case myself. Is this about entr?
Here entr is the change-triggered usage. Most Revise users use non-change triggered Revise I think. (Unless I've misunderstood.)
e.g.
https://github.com/timholy/Revise.jl/blob/891b739539b0381801a2ec6fd3cc641ab098a615/src/packagedef.jl#L1232-L1245
This means that in this case Revise.revise() is run before the user's code is executed in the terminal. This means it is triggered by the user pressing enter rather than a file change.
Here is my reasoning in a bit more depth: My understanding is that the short sleep is really a heuristic to deal with the situation that changes to the filesystem are often non-atomic in practice and bursty like so:
| is a fs change
- is nothing
-------|||-----------------------------|||------------------------------|||------
In the case of entr, we might be woken up directly following a file change. This means there we believe it is reasonably likely we are at the beginning of a burst. Like so:
Case: change triggered
| is a fs change
- is nothing
X We are here
-------|||-----------------------------X||------------------------------|||------
We try and fix this by consolidating the multiple events into a single one. This is sometimes called debouncing. There are different ways we could debounce, but given that each burst is probably short, and they have relatively long gaps between, just waiting a short time after the first one seems reasonable. It might be that this is not logically part of the job really the job of Revise.revise() and should be taken care of somewhere else.
In the case of e.g. revise_first and the REPL, (non-change triggered) I propose that we don't really know where we are in the filesystem change event stream, since we've been triggered by something else:
Case: non-change triggered
| is a fs change
- is nothing
X We are here
-------|||-----------------------------|||------------------------------|||------
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX (Uniformly random between these)
In this case, the short sleep doesn't appear to help.
Note also that the larger context of this issue (debouncing of fs-changes) has to be considered together with https://github.com/timholy/Revise.jl/issues/837 . The current approach of sleeping during the not waiting period seems to depends on later events in the filesystem burst being lost.