Re-implement lazy vals.

[odersky]

• Do a prototype implementation of lazy vals along the lines of #6979.
• Validate its correctness.
• Benchmark its performance.

[liufengyun]

Once this is implemented, we should address one TODO in the following PR:

https://github.com/lampepfl/dotty/pull/9181/files#diff-8dc5ee2dbf11efaad4326519edfa4e45R132

The problem is that the current encoding assumes bitmap fields are non-static, thus in #9181 we cannot mark all fields of module classes as static.

[olsdavis]

Hi, I'm currently working on this, assuming the latest scheme is in this file, in the heading comment. Implementing it, I have realized I would need to sort a few things out (I will be referring to lines in the code I linked):

• in case Evaluating, why is the CAS performed on x instead of _x? I guess it's a small typo;
• in case current: Waiting, why is _x assigned to current.awaitRelease(), given that the latter returns Unit? I believe this is a trace of the first version of the new scheme;
• in case null, how to assign the special value NULL to result, given it is of type A? I have assumed that Evaluating and NULL could be represented as runtime objects, leveraging the AnyRef type of _x like the class Waiting does; but maybe there is a better way to represent these states. If using objects isn't an issue, then maybe we could/should change result to an AnyRef.

[olsdavis]

Hi, I've also noticed that PatternMatcher comes before LazyVals, so I guess I will have to express it directly with if s.

[olsdavis]

I'm having some trouble with the following:

Unsafe's compareAndSwapObject seems to be much more adapted for object fields: its first argument must be the instance containing the field to perform the CAS on. It seems that passing the class object (i.e. TheClass.class in Java) as a first argument works in order to update a static field (that isn't of a primitive type); however, this answer suggests that an extra Object obtained by calling staticFieldBase should be used to safely access a static field when using CAS. (It also points out the issue that some Unsafe methods have been replaced since Java 9, which creates another problem for maintainability.)

Therefore, should an extra field be created to store this extra Object for static lazy vals? Or maybe a static field can be avoided in the first place?

[odersky]

You mean for local lazy vals? I think we might need to box them, which means we have an enclosing object for them. Note that the current lazyvals implementation does the same thing.

[anatoliykmetyuk]

@olsdavis I remember you were saying you may have some time this year to work on this - just checking with you quickly to see if that's the case?

[olsdavis]

Hi, @anatoliykmetyuk. I'm back on it.

The only issue I'm having is that I want to write this Java expression in Scala: Class<?> c = CurrentClass.class;. I tried using classOf[CurrentClass] from Predef, but when I try invoking it, it claims that the function takes no type parameter. I guess it is because type parameters get erased at a previous stage. So how can I refer to the class object of a symbol?

Why I am asking that, it is because the first argument of the CAS is the instance "over which" the CAS should be done. In static context, giving the class instead of an instance works.

Therefore, I also need to know how to determine whether a field is going to be "static". (So I can know what to CAS over.)

[smarter]

Instead of generating a classOf tree you can use tpd.clsOf which will generate the appropriate tree for the current compiler phase.

[olsdavis]

All Tasty tests seem to pass. However, some CompilationTests fail. Namely, i1692.

I must say that I do not understand why it expects the results it requires. Does anyone have more knowledge about this specific test?

Basically, there is the following class:

class LazyNullable(a: => Int) {
  lazy val l0 = a // null out a
  // ...
}

It is instantiated with 'A'.toInt and the test expects that 1. l0 == 'A'.toInt (reasonable), 2. a is null. The second assertion fails with my code, but why would a be null anyway? Is it some odd semantic of lazy vals? If so, is it needed?

[smarter]

Yes, this is needed to avoid memory leaks, it's only done if a is private and the compiler is sure that it isn't used anywhere else, see the pr where this was impleemnted in dotty (scala 2 did the same before): https://github.com/lampepfl/dotty/pull/4289

[sjrd]

This is needed for memory management. We should release references to a as soon as possible so that any memory that a holds on to can be reclaimed by the GC. To do that, there is an overall guarantee that:

• If there is a private[this] field f, and
• f is only read in the body of a (unique) lazy val x,
• then after x is computed, f is zeroed out (nulled out)

[olsdavis]

OK, thanks a lot. I think I am almost done.

[olsdavis]

Hi, so, when I last wrote I just had a few small issues with the tests, as almost all pass except for the following:

• DottyBytecodeTests.scala verifies whether the size of the generated bytecode is equal to some value, but I have changed the way the offset is stored to just a simple long variable. Should I re-apply the same "compression" that was applied to fix this? Even if I do so, I don't believe that the bytecode will be equal in size anyway, or should it? If not, then probably the test should be changed, I guess;
• The test 7723_1.scala fails because of a name conflict 7721_1.scala, in tests/run/alpha-modules-2. I don't believe it has anything to do with my code, but why is it so? Is it probably just a local installation issue?

Thanks in advance!

[odersky]

I am not sure about the DottyBytecodeTests issue. What test fails, and what compression are we talking about?

The 7723_1.scala failure is probably a local installation issue. You can try deleting all class files in your output directory, that is likely to fix the problem.

[olsdavis]

A clean build did fix 7723_1.scala, thank you.

Sorry for the lack of precision: in DottyBytecodeTests, the test lazyFields verifies that the number of instructions in the end bytecode generated by the thread-safe lazy val is equal to 94. I believe this constant should be updated, shouldn't it? (The new answer would be 126.)

I am glad to announce that the latter is actually the only test not to pass!

[odersky]

Sorry for the lack of precision: in DottyBytecodeTests, the test lazyFields verifies that the number of instructions in the end bytecode generated by the thread-safe lazy val is equal to 94. I believe this constant should be updated, shouldn't it? (The new answer would be 126.)

Yes, a difference would be expected here. The only question to ask is whether we are comfortable with the increase in code size. But that's best done by a discussion on a PR.

Congrats and thank you for getting this far! Looking forward to see the PR.

[SethTisue]

superseded by #15296

[dwijnand]

This is the issue that #15296 fixes.

[SethTisue]

I'm sorry, I don't know how I confused that.

[som-snytt]

I'll take this opportunity to say that I totally get the confusion. I often have to pause and ask is this the issue or the PR? and I've seen others mix them up when commenting, as well.

Sorry for the O/T observation that github needs a threading model.

I want "one-stop shopping" for the comments and abandoned PRs for a topic.

To complement the threading model, I want a "needle" feature that will filter for just the current PR.