Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move SafeLazy to the compiler-interface project? #113

Closed
smarter opened this issue May 2, 2016 · 2 comments
Closed

Move SafeLazy to the compiler-interface project? #113

smarter opened this issue May 2, 2016 · 2 comments
Assignees
@jvican

Comments

@smarter
Copy link
Contributor

smarter commented May 2, 2016 •
edited
Loading

I can't figure out why SafeLazy is in a separate project (zinc-apiinfo in sbt 1.0, api in sbt 0.13) and needs to be loaded using reflection from the compiler bridge which assumes that it is present on the classpath:

zinc/internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala

Lines 139 to 144 in 088a577

// call back to the xsbti.SafeLazy class in main sbt code to construct a SafeLazy instance
// we pass a thunk, whose class is loaded by the interface class loader (this class's loader)
// SafeLazy ensures that once the value is forced, the thunk is nulled out and so
// references to the thunk's classes are not retained. Specifically, it allows the interface classes
// (those in this subproject) to be garbage collected after compilation.
private[this] val safeLazy = Class.forName("xsbti.SafeLazy").getMethod("apply", classOf[xsbti.F0[_]])

It seems like it would be much simpler to have SafeLazy in compiler-interface so that no reflexive call is needed. I don't think that this would affect what classes and classloaders can be GCed, but maybe I'm missing something.
@eed3si9n
Copy link
Member

eed3si9n commented May 2, 2016

ba7b6e4 says:

support lazy arguments in data type generator

api subproject is on the sbt side, so it's in Scala 2.11 (or 2.10 for 0.13). From the original commit, I think the main purpose was to expose sbt's lazy value to the compiler-interface, which is Java.
The compiler bridge's job is to run as the Scala version X the build user selected and extract some information in the form of sbt's API datatype in Scala 2.11 (or 2.10 for 0.13), so the overall contortion makes sense to me.

Are you saying we should try to implement something like lazy value in Java?

@smarter
Copy link
Contributor Author

smarter commented May 2, 2016

Yeah I think so, we already have xsbti.F0 so it should be pretty easy.

jvican added a commit to scalacenter/zinc that referenced this issue Feb 7, 2017
@jvican
Fix sbt#113: Implement SafeLazy in Java
c1f821b 
`SafeLazy` has been traditionally implemented in `zincApiInfo` because
it is part of the sbt API and is accessible to all the subprojects that
depend on it.

Before this commit, `SafeLazy` was a runtime dependency (using
reflection) of the compiler bridge. In this regard, Zinc was assuming
that the sbt API was accessible at runtime and therefore invoked it to
use an implementation of lazy that would remove references to the thunks
once they've been forced. This was done to free memory as soon as
possible since those thunks usually depend on classes of compiler
internals and would not be GC'ed otherwise.

However, the compiler bridge is not supposed to depend on sbt APIs since
its code is compiled by the Scala compiler that the user picks in SBT.
Its only dependency is the compiler interface, which is implemented in
Java and compiled beforehand with javac.

This commit removes the runtime dependency of the compiler bridge to the
sbt API and avoids the method invocations using reflection. This was
done for the following reasons:

* Simplicity. It is not obvious why `SafeLazy` is invoked reflectively.
  See sbt#113.
* Performance. Even though the JVM should make this simple use of
  reflection fast, there's a very small overhead of using reflection in
  the compiler bridge because `lzy` is (most likely) hot.

The fix consists of a Java implementation of `SafeLazy` that uses the
non-thread-safe lazy val implementation described [here](http://docs.scala-lang.org/sips/pending/improved-lazy-val-initialization.html).
It is complemented with a proxy written in Scala that will create an
indirection layer for things like by-name and strict evaluation.
This implementation of lazy val assumes that `SafeLazy` will never be
called asynchronously. If this is the case, it's up to the Zinc
maintainer to make sure that safe publishing is implemented at the
call-site or to change the implementation to avoid races and
uninitialized fields.
@jvican jvican mentioned this issue Feb 7, 2017
Merged
eed3si9n added a commit that referenced this issue Feb 7, 2017
@eed3si9n
Merge pull request #221 from jvican/fix-asf
ecb6905 
Fix #113 and apply several other fixes
jvican added a commit to scalacenter/zinc that referenced this issue Feb 26, 2017
@jvican
Fix sbt#113: Implement SafeLazy in Java
16dfbaf 
`SafeLazy` has been traditionally implemented in `zincApiInfo` because
it is part of the sbt API and is accessible to all the subprojects that
depend on it.

Before this commit, `SafeLazy` was a runtime dependency (using
reflection) of the compiler bridge. In this regard, Zinc was assuming
that the sbt API was accessible at runtime and therefore invoked it to
use an implementation of lazy that would remove references to the thunks
once they've been forced. This was done to free memory as soon as
possible since those thunks usually depend on classes of compiler
internals and would not be GC'ed otherwise.

However, the compiler bridge is not supposed to depend on sbt APIs since
its code is compiled by the Scala compiler that the user picks in SBT.
Its only dependency is the compiler interface, which is implemented in
Java and compiled beforehand with javac.

This commit removes the runtime dependency of the compiler bridge to the
sbt API and avoids the method invocations using reflection. This was
done for the following reasons:

* Simplicity. It is not obvious why `SafeLazy` is invoked reflectively.
  See sbt#113.
* Performance. Even though the JVM should make this simple use of
  reflection fast, there's a very small overhead of using reflection in
  the compiler bridge because `lzy` is (most likely) hot.

The fix consists of a Java implementation of `SafeLazy` that uses the
non-thread-safe lazy val implementation described [here](http://docs.scala-lang.org/sips/pending/improved-lazy-val-initialization.html).
It is complemented with a proxy written in Scala that will create an
indirection layer for things like by-name and strict evaluation.
This implementation of lazy val assumes that `SafeLazy` will never be
called asynchronously. If this is the case, it's up to the Zinc
maintainer to make sure that safe publishing is implemented at the
call-site or to change the implementation to avoid races and
uninitialized fields.
eed3si9n pushed a commit to scala/compiler-interface that referenced this issue Apr 23, 2019
@jvican
Fix #113: Implement SafeLazy in Java
f8a802e 
`SafeLazy` has been traditionally implemented in `zincApiInfo` because
it is part of the sbt API and is accessible to all the subprojects that
depend on it.

Before this commit, `SafeLazy` was a runtime dependency (using
reflection) of the compiler bridge. In this regard, Zinc was assuming
that the sbt API was accessible at runtime and therefore invoked it to
use an implementation of lazy that would remove references to the thunks
once they've been forced. This was done to free memory as soon as
possible since those thunks usually depend on classes of compiler
internals and would not be GC'ed otherwise.

However, the compiler bridge is not supposed to depend on sbt APIs since
its code is compiled by the Scala compiler that the user picks in SBT.
Its only dependency is the compiler interface, which is implemented in
Java and compiled beforehand with javac.

This commit removes the runtime dependency of the compiler bridge to the
sbt API and avoids the method invocations using reflection. This was
done for the following reasons:

* Simplicity. It is not obvious why `SafeLazy` is invoked reflectively.
  See sbt/zinc#113.
* Performance. Even though the JVM should make this simple use of
  reflection fast, there's a very small overhead of using reflection in
  the compiler bridge because `lzy` is (most likely) hot.

The fix consists of a Java implementation of `SafeLazy` that uses the
non-thread-safe lazy val implementation described [here](http://docs.scala-lang.org/sips/pending/improved-lazy-val-initialization.html).
It is complemented with a proxy written in Scala that will create an
indirection layer for things like by-name and strict evaluation.
This implementation of lazy val assumes that `SafeLazy` will never be
called asynchronously. If this is the case, it's up to the Zinc
maintainer to make sure that safe publishing is implemented at the
call-site or to change the implementation to avoid races and
uninitialized fields.
eed3si9n pushed a commit to eed3si9n/scala that referenced this issue May 14, 2019
@jvican
Fix scala#113: Implement SafeLazy in Java
573ca00 
`SafeLazy` has been traditionally implemented in `zincApiInfo` because
it is part of the sbt API and is accessible to all the subprojects that
depend on it.

Before this commit, `SafeLazy` was a runtime dependency (using
reflection) of the compiler bridge. In this regard, Zinc was assuming
that the sbt API was accessible at runtime and therefore invoked it to
use an implementation of lazy that would remove references to the thunks
once they've been forced. This was done to free memory as soon as
possible since those thunks usually depend on classes of compiler
internals and would not be GC'ed otherwise.

However, the compiler bridge is not supposed to depend on sbt APIs since
its code is compiled by the Scala compiler that the user picks in SBT.
Its only dependency is the compiler interface, which is implemented in
Java and compiled beforehand with javac.

This commit removes the runtime dependency of the compiler bridge to the
sbt API and avoids the method invocations using reflection. This was
done for the following reasons:

* Simplicity. It is not obvious why `SafeLazy` is invoked reflectively.
  See sbt/zinc#113.
* Performance. Even though the JVM should make this simple use of
  reflection fast, there's a very small overhead of using reflection in
  the compiler bridge because `lzy` is (most likely) hot.

The fix consists of a Java implementation of `SafeLazy` that uses the
non-thread-safe lazy val implementation described [here](http://docs.scala-lang.org/sips/pending/improved-lazy-val-initialization.html).
It is complemented with a proxy written in Scala that will create an
indirection layer for things like by-name and strict evaluation.
This implementation of lazy val assumes that `SafeLazy` will never be
called asynchronously. If this is the case, it's up to the Zinc
maintainer to make sure that safe publishing is implemented at the
call-site or to change the implementation to avoid races and
uninitialized fields.
lrytz pushed a commit to lrytz/scala that referenced this issue Nov 5, 2019
@jvican
Fix scala#113: Implement SafeLazy in Java
5417cb0 
`SafeLazy` has been traditionally implemented in `zincApiInfo` because
it is part of the sbt API and is accessible to all the subprojects that
depend on it.

Before this commit, `SafeLazy` was a runtime dependency (using
reflection) of the compiler bridge. In this regard, Zinc was assuming
that the sbt API was accessible at runtime and therefore invoked it to
use an implementation of lazy that would remove references to the thunks
once they've been forced. This was done to free memory as soon as
possible since those thunks usually depend on classes of compiler
internals and would not be GC'ed otherwise.

However, the compiler bridge is not supposed to depend on sbt APIs since
its code is compiled by the Scala compiler that the user picks in SBT.
Its only dependency is the compiler interface, which is implemented in
Java and compiled beforehand with javac.

This commit removes the runtime dependency of the compiler bridge to the
sbt API and avoids the method invocations using reflection. This was
done for the following reasons:

* Simplicity. It is not obvious why `SafeLazy` is invoked reflectively.
  See sbt/zinc#113.
* Performance. Even though the JVM should make this simple use of
  reflection fast, there's a very small overhead of using reflection in
  the compiler bridge because `lzy` is (most likely) hot.

The fix consists of a Java implementation of `SafeLazy` that uses the
non-thread-safe lazy val implementation described [here](http://docs.scala-lang.org/sips/pending/improved-lazy-val-initialization.html).
It is complemented with a proxy written in Scala that will create an
indirection layer for things like by-name and strict evaluation.
This implementation of lazy val assumes that `SafeLazy` will never be
called asynchronously. If this is the case, it's up to the Zinc
maintainer to make sure that safe publishing is implemented at the
call-site or to change the implementation to avoid races and
uninitialized fields.

Rewritten from sbt/zinc@c1f821b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jvican jvican

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@smarter @eed3si9n @jvican