Skip to content

Conversation

@mbovel
Copy link
Member

@mbovel mbovel commented Dec 3, 2025

No description provided.


/** Get the bytes of the given tasty file, using the cache if possible. */
def getTastyBytes(tastyFile: AbstractFile): Array[Byte] =
tastyBytesCache.synchronized:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is an AbstractFile a good key for a weakhashmap? i.e. the same exact AbstractFile will be used over and over? I would think perhaps between runs the abstract file is GC'd possibly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it depends on how dotty's zipclasspath does its own caching?

Copy link
Member Author

@mbovel mbovel Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it depends on how dotty's zipclasspath does its own caching?

Yes exactly. This relies on classpaths being cached here:

/**
* A trait providing an optional cache for classpath entries obtained from zip and jar files.
* It allows us to e.g. reduce significantly memory used by PresentationCompilers in Scala IDE
* when there are a lot of projects having a lot of common dependencies.
*/
sealed trait ZipAndJarFileLookupFactory {
private val cache = new FileBasedCache[ClassPath]
def create(zipFile: AbstractFile)(using Context): ClassPath =
val release = Option(ctx.settings.javaOutputVersion.value).filter(_.nonEmpty)
if (ctx.settings.YdisableFlatCpCaching.value || zipFile.file == null) createForZipFile(zipFile, release)
else createUsingCache(zipFile, release)
protected def createForZipFile(zipFile: AbstractFile, release: Option[String]): ClassPath
private def createUsingCache(zipFile: AbstractFile, release: Option[String]): ClassPath =
cache.getOrCreate(zipFile.file.toPath, () => createForZipFile(zipFile, release))
}

Only in that case do we get stable AbstractFiles instances across runs.

I tried that for two reasons:

  • Absolute path and modified time are not enough as keys; we can get files with different content but same absolute path and modified time. It happens when loading classes from the JRT at least (that's why [Experiment] Global file content cache #24630 fails).
  • It's a good way to avoid memory leaks: we only retains content for AbstractFiles that are reachable. Otherwise the WeakMap will let the GC do its job, which is what we want! That's similar to Cache AbstractFile.toByteArray #24644, but with an external map instead of using a field. So we avoid caching too much with this solution, but the drawback of course is that we risk caching too little.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants