Fix overly long mtime change delay calibration with HFS+.

[23Skidoo]

Fixes #4230.

[mention-bot]

@23Skidoo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ezyang, @phadej and @sol to be potential reviewers.

[ezyang]

What's the testing plan for this? Can we mock file system time interface? What happens if the resolution is 0.999s?

[23Skidoo]

What's the testing plan for this? Can we mock file system time interface?

I can do this, but seems more trouble than it's worth. Maybe just ask @gracjan to test on his system?

What happens if the resolution is 0.999s?

We can change the check to mtime0 >= 1000000 || (1000000 - mtime0) <= 10000.

[ezyang]

Legacy code is code without an automated test suite ;) But, we do have Mac OS X CI setup, so we could just test to make sure the calibration time doesn't take a wacky amount of time.

Isn't that a more reasonable metric? We don't want to spend too much time calibrating, so use that as the stopping point?

[23Skidoo]

But, we do have Mac OS X CI setup, so we could just test to make sure the calibration time doesn't take a wacky amount of time.

OK, will do.

[ezyang]

BTW, this is well worth solving; I bet we are chewing up a decent amount of time in our Mac OS X CI recalibrating here!

[23Skidoo]

Added a test.

[23Skidoo]

@ezyang You were quite right, apparently my fix doesn't actually work on OS X. Will try increasing the epsilon and/or following @gracjan's suggestions.

[23Skidoo]

Increasing the epsilon to 0.1 s seems to do the trick... will now try @gracjan's suggestion.

[ezyang]

Still failing Travis on OS X, it's real.

[gracjan]

The list of mtimes measured in the function looks like this:

*Distribution.Backpack Distribution.Compat.Time> calibrateMtimeChangeDelay 
[335264,994224,999491,1000088,999906,1000045,999953,1000012,999869,1001424,1003468,995152,999844,1000068,999953,1000066,1000889,998953,1003979,996114,1238135,761660,1000062,1006898,993069]
(1238135,1000000)

With bail-out after first measurement that gives 1s or more the list will be [335264, 994224, 999491, 1000088] and that is 3329067 (3.3s).

I'd like to make clear that '@gracjan's suggestion' is to measure file system resolution not clock resolution and that should take on average 0.5s on 1s resolution filesystem.

[23Skidoo]

Remilestoned.

[Kleidukos]

Marking this PR as draft 🙂