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.

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

Revert Stream.Cons to the 2.12 encoding #8354

Merged
merged 1 commit into from Aug 26, 2019
Merged

Conversation

szeiger
Copy link
Member

@szeiger szeiger commented Aug 20, 2019

  • The lazy val required an extra bitmap field which increases the
    instance size from 24 to 32 bytes.
  • Capturing tl in the initialization code prevented it from being
    garbage collected as soon as the tail has been computed and the
    initializer is no longer needed.

Fixes scala/bug#11696, at least in theory.
I was unable to reproduce the failure with the code from the original
bug report.

LazyList is not affected. It already releases the initializer after
running it, and the instance size (on x64) is 24 bytes with or without
the bitmap field for the lazy val.

@scala-jenkins scala-jenkins added this to the 2.13.2 milestone Aug 20, 2019
@szeiger szeiger modified the milestones: 2.13.2, 2.13.1 Aug 20, 2019
@szeiger
Copy link
Member Author

szeiger commented Aug 20, 2019

I'm not sure what to make of the Dotty error:

[error] -- [E099] Syntax Error: /home/travis/build/scala/scala/src/library/scala/collection/immutable/Stream.scala:376:43 
[error] 376 |    @volatile private[this] var tlGen = tl _
[error]     |                                        ^^^^
[error]     |Only function types can be followed by _ but the current expression has type Stream[A]

This is the same as 2.12 and it's also found in the Dotty repo: https://github.com/lampepfl/dotty/blob/master/tests/scala2-library/src/library/scala/collection/immutable/Stream.scala#L1163

@allanrenucci
Copy link
Contributor

It is a warning under -language:Scala2. You would need to write:

@volatile private[this] var tlGen = () => tl

- The `lazy val` required an extra bitmap field which increases the
  instance size from 24 to 32 bytes.
- Capturing `tl` in the initialization code prevented it from being
  garbage collected as soon as the tail has been computed and the
  initializer is no longer needed.

Fixes scala/bug#11696, at least in theory.
I was unable to reproduce the failure with the code from the original
bug report.

LazyList is not affected. It already releases the initializer after
running it, and the instance size (on x64) is 24 bytes with or without
the bitmap field for the lazy val.
Copy link
Contributor

@NthPortal NthPortal left a comment

Choose a reason for hiding this comment

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

LGTM

@szeiger szeiger merged commit a45faa3 into scala:2.13.x Aug 26, 2019
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
5 participants