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

More elegant holders for local lazy vals. #5374

Merged
merged 1 commit into from Sep 3, 2016

Conversation

Projects
None yet
5 participants
@adriaanm
Member

adriaanm commented Sep 2, 2016

Follow up for #5294 based on @retronym's feedback.

I'm going to defer teaching the optimizer about stack allocating the lazy holders: scala/scala-dev#215

@adriaanm adriaanm added this to the 2.12.0-RC1 milestone Sep 2, 2016

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Sep 2, 2016

Member

LGTM!

Member

lrytz commented Sep 2, 2016

LGTM!

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Sep 3, 2016

Member

LGTM

Member

retronym commented Sep 3, 2016

LGTM

@retronym retronym merged commit 04c7fae into scala:2.12.x Sep 3, 2016

5 checks passed

cla @adriaanm signed the Scala CLA. Thanks!
Details
integrate-ide [3177] SUCCESS. Took 16 s.
Details
validate-main [3580] SUCCESS. Took 75 min.
Details
validate-publish-core [3511] SUCCESS. Took 5 min.
Details
validate-test [3092] SUCCESS. Took 64 min.
Details

@adriaanm adriaanm added the 2.12 label Oct 29, 2016

class LazyRef[T] {
var value: T = _
@volatile var initialized: Boolean = false
@volatile private[this] var _initialized: Boolean = _

This comment has been minimized.

@viktorklang

viktorklang Mar 15, 2017

Contributor

Why is this @volatile btw?

@viktorklang

viktorklang Mar 15, 2017

Contributor

Why is this @volatile btw?

This comment has been minimized.

@adriaanm

adriaanm Mar 20, 2017

Member

Quoting @retronym:

In double checked locking, the write to the value is in a synchronized block
the fast path (already initialized) just needs to volatile read on the initialized field. if that is set, we know the value was safely published

@adriaanm

adriaanm Mar 20, 2017

Member

Quoting @retronym:

In double checked locking, the write to the value is in a synchronized block
the fast path (already initialized) just needs to volatile read on the initialized field. if that is set, we know the value was safely published

This comment has been minimized.

@viktorklang

viktorklang Mar 20, 2017

Contributor

Apologies, I guess what threw me of course is that this class (or rather these classes) are public, and non-final, classes which cannot be used properly without sprinkling of "magic"/macros. Are these intended to be possible to use directly by users?

@viktorklang

viktorklang Mar 20, 2017

Contributor

Apologies, I guess what threw me of course is that this class (or rather these classes) are public, and non-final, classes which cannot be used properly without sprinkling of "magic"/macros. Are these intended to be possible to use directly by users?

This comment has been minimized.

@adriaanm

adriaanm Mar 20, 2017

Member

True, the runtime package is not meant for public consumption. Good point
that they should likely be final. They have to be public though, as they
are referenced from compiler-generated code.

scala/scala-dev#339

@adriaanm

adriaanm Mar 20, 2017

Member

True, the runtime package is not meant for public consumption. Good point
that they should likely be final. They have to be public though, as they
are referenced from compiler-generated code.

scala/scala-dev#339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment