Millisecond-precision file modification timestamps using native code #92
Conversation
|
Please do not merge yet, I will use the pull requests to perform additional validations |
|
The linked ticket states that the Files method works correctly. Is that not true? |
|
I added the link to JDK-8177809. @cunei Could you check this? |
|
If you are referring to |
|
@cunei Yup that's the one. Thanks for the checking. |
|
If anyone reading this want to try it at home. From sbt shell, run: |
|
I checked on macOS with APFS and Java 10: Welcome to Scala 2.12.4 (Java HotSpot(TM) 64-Bit Server VM, Java 10-ea).
Type in expressions for evaluation. Or try :help.
scala> import java.io.File
import java.io.File
scala> import java.nio.file.Files
import java.nio.file.Files
scala> val f = new File("Downloads")
f: java.io.File = Downloads
scala> Files.getLastModifiedTime(f.toPath).toMillis
res0: Long = 1512376609000
scala> f.lastModified
res1: Long = 1512376609808So, it looks like |
|
I asked in the nio-dev mailing list and Alan clarified:
|
57d2770 to
c1ecc7c
Compare
|
Travis fails because of compiler-bridge_2.13.0-M2:1.0.5, but the rest seems otherwise ok. |
|
The five commits concerning millisecond timestamps should be good to go; a unit test is still missing, but right now I would proceed with the merge in order to unblock Dale; also, there is no practical way to write a unit test that covers the various platforms: by definition I can only test the native calls on the architecture in use at build time. The startup diagnostic is also intended to warn if native timestamps do not work as intended (see sbt/sbt#3789 (comment))
|
project/build.properties
Outdated
| @@ -1 +1 @@ | |||
| sbt.version=1.0.0 | |||
| sbt.version=1.0.4 | |||
There was a problem hiding this comment.
sbt 1.0.4 is unable to build sbt 2.13.0-M1, so could you exclude it from Travis CI yml?
There was a problem hiding this comment.
Right, that's the issue. I'll drop the commit that updates sbt.version.
|
Btw, is this a significant enough issue that it's worth keeping native code around even though it has been fixed in the JDK itself? |
Reverted sbt.version, all checks passed.
|
The fix is scheduled for inclusion only in JDK 10, but we will need to support JDK 8 and JDK 9 for many years to come. Also, this issue caused some of our tests to fail non-deterministically, and caused a bunch of other collateral issues. So, yep, we really ought to fix this. |
|
JDK 9 will be EOL'd by Oracle in March when JDK 10 ships (it's not a LTS release), so I'd be surprised if anyone supports it for a long time. JDK 8 will be around for a while, I agree. However, people have the option of running SBT with JDK 10+ while supporting older JDK versions via the Anyway, you are the ones maintaining the code so your call. :) |
|
We came across this behavior during integration testing, but we also suspect it's been affecting other areas in sbt such as triggered execution and incremental compilation. |
|
@cunei Thanks for all the ripple PRs. I'm going to run Dbuild and see what happens. |
See [[JDK-8177809] File.lastModified() is losing milliseconds (always ends in 000)](https://bugs.openjdk.java.net/browse/JDK-8177809). The Java methods `lastModified()` and `getLastModifiedTime()`, on many JDKs and operating systems, return a number rounded to the whole second, losing the millisecond part. Since those timestamps are used by sbt to detect file changes, the result is that rapid file changes may go undetected, leading to mysterious errors. This pull request uses JNA (a native code interface) to replace `lastModified()` with native calls that return timestamps with full millisecond precision, on Linux 64-bit, Linux-32bit, Windows, and OSX. The new methods are called `getModifiedTime()` and `setModifiedTime()`. The code will fall back to Java if no native support is available, or if the property "sbt.io.jdktimestamps" is defined and is not `"false"`. A diagnostic method `sbt.internal.io.Milli.getMilliSupportDiagnostic(projectDir: File): Option[String]` is supplied to check whether the filesystem/JDK/OS in use support sub-second modification timestamps. It is intended to be run once after the project is loaded, with something like: ``` sbt.internal.io.Milli.getMilliSupportDiagnostic(projectDir) map log.debug ``` If the test fails, it returns a diagnostic string that will remain available in the logs.
Also preserve the current behavior of IO.copyLastModified(), for compatibility. The method IO.copyLastModified() has an underspecified, and generally inconsistent behavior. In particular, if the source file is missing, it will silently set the target modification time to 1st January 1970, returning success. Also, because it uses lastModified(), it may trim away the millisecond part of the timestamp without notice. The replacement copyModifiedTime(), instead, has true millisecond precision. Also, it will correctly throw a FileNotFoundException if any of the two files are missing, or an IOException in case an IO exception occurs. This commit deprecates copyLastModified() in favor of the new copyModifiedTime(), adding a Scaladoc explanation.
|
As per discussion during sbt 1 meeting, I am retargeting this to 1.1.x branch. |
|
The dbuild validation ran up until the vscode stuff, which is not yet in 1.0.x so I'd consider it a pass. |
|
I temporarily used as base I can keep bypassing the exception, but inspecting files that do not actually exist is probably not a great idea; I'd rather check why that happens, and fix the client code instead. OK? @eed3si9n |
|
Cool! However I recommend having a pure jvm fallback in case of exception. Your detection logic could fail, or abis could change, then we're basically not on the jvm anymore. |
|
Also, are you sure JNA isn't adding a performance bottleneck? These are very frequent calls and even submilli slowdowns will become apparent on large codebase as multisecond slowdowns. We really need JNI bulk operations to avoid problems. But does it also suffer from precision problems? I'd personally prefer performance over precision in this case, note that we cannot say "correctness" because we're still limited by the filesystem precision. |
See [JDK-8177809] File.lastModified() is losing milliseconds (always ends in 000).
The Java methods
lastModified()andgetLastModifiedTime(), on many JDKs and operating systems, return a number rounded to the whole second, losing the millisecond part. Since those timestamps are used by sbt to detect file changes, the result is that rapid file changes may go undetected, leading to mysterious errors.This pull request uses JNA (a native code interface) to replace
lastModified()with native calls that return timestamps with full millisecond precision, on Linux 64-bit, Linux 32-bit, Windows, and macOS. The new methods are calledgetModifiedTime()andsetModifiedTime(). The code will fall back to Java if no native support is available, or if the property "sbt.io.jdktimestamps" is defined and is not"false".A diagnostic method
sbt.io.Milli.getMilliSupportDiagnostic(projectDir: File): Option[String]is supplied to check whether the filesystem/JDK/OS in use support sub-second modification timestamps. It is intended to be run once after the project is loaded, with something like:If the test fails, it returns a diagnostic string that will remain available in the logs.