Use classpath PathRefs hashCode as cache key for Zinc worker #2185
Conversation
Since the compiler jars have the Scala version in the name, it is enough The `compilersSig` takes now ~20ms vs ~200ms to build in my machine Since it runs on every recompilation, it makes the compiler noticeably faster
First, my feeling about this is not good. The whole correctness is based on assumptions we don't check or enforce. You need a counter example? Yeah, probably seldom, but you could use a locally built compiler jar, which might be always in the same place and lack any indicator of change in it's name. Whenever you rebuild it, you probably also should update the bridge. Or you download or build a custom compile in another Mill target and return it as As a consequence, risking the whole correctness on an weak assumption for "only" some milliseconds isn't worth it. But, as we already on this issue, I have another idea to offer. We already know much more about those compiler classpath that the file names, the |
| val compilersSig = | ||
| compilerBridgeSig + | ||
| combinedCompilerClasspath.map(p => p.toString().hashCode + os.mtime(p)).sum | ||
| val compilersSig = combinedCompilerClasspath.map(p => p.hashCode).sum |
There was a problem hiding this comment.
This will miss changes to the files.
|
I implemented the approach you suggested and now we also perform much fewer conversions. |
| )(f: Compilers => T)(implicit ctx: ZincWorkerApi.Ctx) = { | ||
| val combinedCompilerClasspath = compilerClasspath ++ scalacPluginClasspath | ||
| val combinedCompilerJars = combinedCompilerClasspath.iterator.toArray.map(_.toIO) | ||
| val compilersSig = combinedCompilerClasspath.hashCode + scalaVersion.hashCode + scalaOrganization.hashCode |
There was a problem hiding this comment.
Was there some issue you fixed by adding the scala version and organization, or is this just for completeness?
There was a problem hiding this comment.
The previous code had compilerBridgeSig which was calculated also with:
val compilerBridgeSig = os.mtime(compiledCompilerBridge)Since compiledCompilerBridge is calculated using scalaVersion, scalaOrganization and compilerClasspath I took the hashCode of the input, so I added scalaVersion and scalaOrganization to the result.
compilerClasspath is already taken into the sig with the hashCode of combinedCompilerClasspath
lolgab
changed the title
|
Updated the description and collected numbers in a better way :) |
The previous implmentation was receiving
os.Paths so it needed to access the filesystem to know if the files did change.Now we propagate
PathRefs so we simply use theirhashCodewhich changes if the files did change.Here some numbers collected by compiling a Scala file 176 times and writing down the time calculating
compilersSigtook.