Skip to content

-Ymacro-classpath / Global.close() / Reference counted FileBasedCache #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from

Conversation

retronym
Copy link
Owner

No description provided.

@retronym retronym force-pushed the topic/macro-classloader branch from 634c001 to e2a829f Compare October 15, 2018 09:50
inputStream.close()
}
case None =>
Failure(new MissingPluginException(path))
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

@som-snytt I'm thinking of making this change to read scalac-plugin.xml from a classloader, rather than by looking through JARs and directories. The motivation is to let customized Global-s provide the classloader, to do things like manage the lifecycle of the classloader + open file handles with extra knowledge of multi-project builds.

Seems to me like a logical extension of the groundwork you laid in scala#3169. WDYT?

Choose a reason for hiding this comment

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

Sounds good. I don't remember why the old logic looks so labored. Please indulge my alliteration.

By coincidence, at the day job I just asked to switch some legacy config from a command line option to a class loader resource.

I read the related tickets around class loader caching.

case (j, Success(pd)) => Success((pd, findPluginClassloader(Seq(j))))
val fromLoaders = paths.map {path =>
val loader = findPluginClassloader(path)
val list: Option[URL] = loader.getResources(PluginXML).asScala.take(1).toList.headOption

Choose a reason for hiding this comment

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

Option(loader.getResource(PluginXML)) or is there a mysterious gotcha? Besides they're going to rename it to Option.ofNullable?

Copy link
Owner Author

Choose a reason for hiding this comment

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

@som-snytt Oh yeah, that sounds better. I got too attached to the existing loops.

@retronym retronym changed the title Topic/macro classloader -Ymacro-classpath / Global.close() Oct 17, 2018
This lets customized Global's deliver this file, and offloads
the scanning logic.
@retronym retronym force-pushed the topic/macro-classloader branch 2 times, most recently from 171ad8e to deb10d4 Compare October 17, 2018 04:49
@retronym retronym changed the title -Ymacro-classpath / Global.close() -Ymacro-classpath / Global.close() / Reference counted FileBasedCache Oct 17, 2018
case pathString =>
import java.nio.file._
val path = Paths.get(pathString)
Files.write(path, argsFile.getBytes(Charset.forName("UTF-8")))

Choose a reason for hiding this comment

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

This decade there is StandardCharsets.

@retronym retronym force-pushed the topic/macro-classloader branch from d8bb552 to deb10d4 Compare October 23, 2018 07:22
Close Global-scoped JARs and ClassLoaders.

Entries of FileBasedCache that have a lifetime that exceeds any
single Global instance are reference counted. When the count
hits zero, it is closed if no references appear after a delay.
@retronym retronym force-pushed the topic/macro-classloader branch from deb10d4 to 30c372a Compare October 23, 2018 07:40
@retronym
Copy link
Owner Author

Submitted as scala#7366

@retronym retronym closed this Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants