Bug ID: JDK-8097996 Use Lambda in FX runtime and samples

JDK 8
8u20Fixed

For completeness, the following lambda changeset was also pushed with this bug ID (on Feb 24) to lambdify Ensemble8: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/b02945e62c40
03-09-2014
Yep.
11-06-2014
... and you ran the tests and it all worked?
10-06-2014
To close the loop, here is the patch of Snapshot1Test I generated using Netbeans (I had to revert just the two compile-time failures).
10-06-2014
We are done lambdaizing FX. It was interesting that the different IDE's had different errors and various strengths and weaknesses. Along the way, a few cases exposed some interesting problems where lambda was not equivalent to inner classes. It's a good topic to investigate at a later time and perhaps a good topic for a talk.
10-06-2014
The other two compile and pass the test suite? Interesting.
10-06-2014
When you get a moment (no hurry), could you attach a patch with what IntelliJ tried to do? With NB, I only get the two expected compile-time failures. The other two compile and pass. Just curious.
10-06-2014
Changeset: 79ce29689619 Author: snorthov Date: 2014-06-10 14:35 -0400 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/79ce29689619
10-06-2014
Yes, the outer runnable could have been lamdified but I just reverted the entire method to be simple. There were 4 (2 failed at compile time and 2 others compiled but failed at runtime).
10-06-2014
I see them now. There are two anonymous inner classes in each of the two failing methods. The outer one (runAndWait) can be turned into a lamba with no problem. The inner one cannot since I am explicitly testing something that would fail to compile if generified. I don't know what IntelliJ did with it, but what NetBeans tries to do doesn't even compile. The simplest thing is just to just leave the methods that you reverted and push what you have (although as I noted, the calls to runAndWait could be lambdafied with no problem). +1 for pushing the rt-tests-lambda.patch as is.
10-06-2014
Not off hand. All of the tests should pass. I reverted the two failing methods already. I believe the other one was testBadNodeCallback2(). It's interesting that there was no compile error but the wrong thing happened and the tests failed.
10-06-2014
I'm running the tests now, and I will post results. I am also looking into the failing method. So you have a list of lambda expressions you had to revert?
10-06-2014
One of the methods was testBadSceneCallback2(). We might consider lamdifying and investigating the reason for failure at some point to understand why lamdification produced code that did not work the same. The theory is that lambda and inner classes are mostly equivalent.
10-06-2014
Please apply the patch and run the tests. I did this already and found that lamdifying some of the snapshot code caused test failures. It was quite interesting. I did not investigate but simply backed out the changes in question.
10-06-2014
Hi Kevin, Please review the lambdification of the rt-tests component. You are welcome to apply the patch, but there are numerous changes and they are all automatic. If you have outstanding changes, please commit them and I will lambdify once more. Jira: https://javafx-jira.kenai.com/browse/RT-35197 Webrev: See patch is in the JIRA Steve
10-06-2014
I think only rt/tests/system still needs to be done.
10-06-2014
If we fix RT-36734, then I think we don't need to revert anything. On a side note, our gradle script checks the minimum JDK_HOME version and requires it to be 8b115+. But the check is only performed after the SSEBackend.java compilation. Perhaps we should move the JDK version check a bit earlier. Also, I think it makes sense to require using 8 GA (b132) instead of b115 now.
21-04-2014
Hmmm … we can undo this change. There was only a single method that was lambdified in buildSrc. If somehow allowing buildSrc to use JDK7 allows more flexibility or helps debugging etc., then having a single lambda method in buildSrc prevent this doesn't make sense.
21-04-2014
I've filed RT-36734 to track the JAVA_HOME issue.
18-04-2014
This suggests that something in our build scripts is still using JAVA_HOME when it should be using JDK_HOME.
17-04-2014
OK, I've resolved this issue by specifying both JDK_HOME and JAVA_HOME, and pointing both to a JDK 8 installation. Note that previously only JDK_HOME was required to be Java 8.
17-04-2014
With the last fix I get the following error when starting a build on my Mac: $ JDK_HOME=/usr/java/8b115 gradle Defining Closed Properties :buildSrc:clean :buildSrc:generateGrammarSource :buildSrc:compileJava /net/lin/export/anthony/ws/g-28-twoTransparentWindows-RT-36716/rt/buildSrc/src/main/java/com/sun/scenario/effect/compiler/backend/sw/sse/SSEBackend.java:85: error: ')' expected Comparator<Variable> c = (v0, v1) -> v0.getName().compareTo(v1.getName()); ^ /net/lin/export/anthony/ws/g-28-twoTransparentWindows-RT-36716/rt/buildSrc/src/main/java/com/sun/scenario/effect/compiler/backend/sw/sse/SSEBackend.java:85: error: ';' expected Comparator<Variable> c = (v0, v1) -> v0.getName().compareTo(v1.getName()); ^ 2 errors :buildSrc:compileJava FAILED /usr/java/8b115 is 8b115. The same error occurs if I use JDK 8 GA (b132).
17-04-2014
buildSrc change set: Changeset: d53abe403e7f Author: snorthov Date: 2014-04-16 18:58 -0400 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/d53abe403e7f
16-04-2014
buildSrc patch looks good to go.
16-04-2014
Graphics change set: Changeset: 50c055ca2ce1 Author: snorthov Date: 2014-04-08 10:40 -0400 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/50c055ca2ce1
08-04-2014
One more thing to lambda-ize are the system tests located in: rt/tests/system/**
08-04-2014
I filed RT-36577 to track restoring the missing comments.
08-04-2014
I agree. We can file a new JIRA to restore the missing comments. +1 on the second version of the graphics path.
08-04-2014
I agree that removing comments is evil. Eclipse seemed to do a better job here but had other bugs in the lambda-izer. How to proceed? Are you telling me that the previous lambda-izer left the above expression alone? The latest patch was a two pass effort. I back out and run only a single pass which might leave this expression alone and will be consistent with the way the rest of FX has be lambda-ized so far. Thoughts? Personally I would just go with the patch we have.
08-04-2014
Here is another one, and it was in the first patch, too, so not a results of rerunning the lambda-fication: @@ -64,12 +62,7 @@ GtkApplication() { boolean isEventThread = AccessController - .doPrivileged(new PrivilegedAction<Boolean>() { - public Boolean run() { - // Embedded in SWT, with shared event thread - return Boolean.getBoolean("javafx.embed.isEventThread"); - } - }); + .doPrivileged((PrivilegedAction<Boolean>) () -> Boolean.getBoolean("javafx.embed.isEventThread")); This is a bug in IntelliJ. In no way should the lambda-fication remove comments. How do you want to proceed? Note that I suspect this may have already happened for other modules.
08-04-2014
I looked at the delta diffs between the last version of graphics-lambda and the one you just posted, and found at least one thing that concerns me. IntelliJ removed a comment in the refactoring, which is absolutely the wrong thing for it to do! It makes me kind of suspicious of the rest since I never would have expected that. - PlatformImpl.runAndWait(new Runnable() { - @Override public void run() { -// System.err.println("PlatformImpl.tkExit: calling Toolkit.exit"); - Toolkit.getToolkit().exit(); - } - }, true); + PlatformImpl.runAndWait(() -> Toolkit.getToolkit().exit(), true); Now in this case I suppose one could argue that the comment it removed isn't important, but in general, this is evil.
08-04-2014
Sorry about this Kevin but code has changed and IntelliJ seems to miss lambda opportunities. I have run the tools again and attached a new patch. If we can quickly, we can commit the changes before more code changes <g>. I have run the tests on Mac but not Windows.
08-04-2014
+1 on the graphics changes.
08-04-2014
+1 for the web part
04-04-2014
Controls changeset: Changeset: 342579f8f7e1 Author: snorthov Date: 2014-03-28 14:50 -0400 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/342579f8f7e1
28-03-2014
Controls changeset is massive, but from a visual inspection of the patch I can't see anything wrong. It's a +1 from me.
28-03-2014
Hi Jonathan, Please review the lambdification of the controls component. You are welcome to apply the patch, but there are numerous changes and they are all automatic. If you have outstanding changes, please commit them and I will lambdify once more. Jira: https://javafx-jira.kenai.com/browse/RT-35197 Webrev: See patch is in the JIRA Steve
27-03-2014
Web changeset: Changeset: b873fbebaf0d Author: snorthov Date: 2014-03-13 07:40 -0400 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/b873fbebaf0d
13-03-2014
Changeset: 29346901e6f1 Author: snorthov Date: 2014-03-06 16:20 -0500 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/29346901e6f1
06-03-2014
Media part approved.
06-03-2014
Hi Kirill, Please review the lambdification of your component. You are welcome to apply the patch, but there are numerous changes and they are all automatic. If you have outstanding changes, please commit them and I will lambdify once more, Jira: https://javafx-jira.kenai.com/browse/RT-35197 Webrev: See patch is in the JIRA Steve
05-03-2014
Leonid, you are the owner of Web. Can I get a +1 to release the patch?
05-03-2014
Kevin fixed the code to avoid problem and released the new code. I have relamdified and attached a new patch. The hang is gone.
05-03-2014
Good question. I boiled it down to a simple test case with no FX in the picture. Using an anonymous inner class works. Using an equivalent lambda hangs. I filed: https://bugs.openjdk.java.net/browse/JDK-8036728
05-03-2014
I'm confused. How can changing an inner class into a lambda make this happen?
05-03-2014
I am going to file a JDK bug on the hang when doing this from class init. I can see from the stack trace that class initialization is still in progress on the JUnit worker thread, which is waiting in the countdown latch, while the lamda is also waiting for something. I think the way forward for FX is either to make the change I suggested in my previous comment (which really is a better way to go and is what we do for most of our tests).
05-03-2014
So here is the patch (on top of Steve's patch) that makes the tests work: diff --git a/modules/web/src/test/java/javafx/scene/web/TestBase.java b/modules/web/src/test/java/javafx/scene/web/TestBase.java --- a/modules/web/src/test/java/javafx/scene/web/TestBase.java +++ b/modules/web/src/test/java/javafx/scene/web/TestBase.java @@ -18,6 +18,7 @@ import com.sun.javafx.application.PlatformImpl; import java.util.concurrent.ExecutionException; +import org.junit.BeforeClass; import org.w3c.dom.Document; public class TestBase implements ChangeListener, InvalidationListener { @@ -27,7 +28,8 @@ private static WebView view; - static { + @BeforeClass + public static void setupOnce() { final CountDownLatch startupLatch = new CountDownLatch(1); PlatformImpl.startup(() -> {
05-03-2014
Narrowed it down even further. If the chunk of code in the static { ... } block is moved to a public static method annotated with @BeforeClass (which is a better apprach anyway) then it runs fine. For some reason, the same code works in a static method called after class initialization but doesn't work during class initialization. Very odd.
05-03-2014
Latest JDK made no difference. I have narrowed it down further. Reverting just the one lambda change that is hanging in just this one file allows the test to pass. I don't see anything wrong with the code, so I suspect a JDK bug. diff --git a/modules/web/src/test/java/javafx/scene/web/TestBase.java b/modules/ web/src/test/java/javafx/scene/web/TestBase.java --- a/modules/web/src/test/java/javafx/scene/web/TestBase.java +++ b/modules/web/src/test/java/javafx/scene/web/TestBase.java @@ -30,8 +30,10 @@ static { final CountDownLatch startupLatch = new CountDownLatch(1); - PlatformImpl.startup(() -> { - startupLatch.countDown(); + PlatformImpl.startup(new Runnable() { + public void run() { + startupLatch.countDown(); + } }); try {
05-03-2014
Yes, I can confirm that. If I revert just the lambda changes in the test files in the web module, leaving the src/main/java files lamdified, then the test passes. I was running an older version of the JDK 8, so on the off chance that this is a JDK lambda bug, I am rebuilding with the latest GA candidate build of JDK 8. I'll post results.
05-03-2014
Interesting! Can you confirm that the hang happened because of the lambda changes?
05-03-2014
Btw, I'm not Leonid, but the web changes look good to me. kcr, edited to add: See below for a test failure.
04-03-2014
As an aside, I find it unhelpful that the stack trace says "Unknown Source" for the file:line_number. That seems like a loss of functionality.
04-03-2014
Attached the stack trace. Here is a possibly interesting bit: "JavaFX Application Thread" #15 prio=5 os_prio=0 tid=0x06334c00 nid=0x131c in Object.wait() [0x07b7e000] java.lang.Thread.State: RUNNABLE at javafx.scene.web.TestBase$$Lambda$2/19562965.run(Unknown Source) at com.sun.javafx.application.PlatformImpl$5.run(PlatformImpl.java:219) at com.sun.glass.ui.Application.invokeAndWait(Application.java:458)
04-03-2014
NOTE: I applied and ran the web-lambda.patch on Windows, and one of the unit tests hangs. I will attach a stack trace shortly.
04-03-2014
Hi Leonid, Please review the lambdification of the web component. You are welcome to apply the patch, but there are numerous changes and they are all automatic. If you have outstanding changes, please coordinate with me and I will re-lambdify to include the changes. Jira: https://javafx-jira.kenai.com/browse/RT-35197 Webrev: See patch is in the JIRA Steve
04-03-2014
Swing change set: Changeset: 1e707c916396 Author: snorthov Date: 2014-03-04 10:07 -0500 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/1e707c916396
04-03-2014
Hi Steve, Please consider generating a webrev next time, because patches miss a lot of context that is very useful when reviewing code. I briefly skimmed through the patch and it looks okay. +1.
04-03-2014
Hi Anthony, Please review the lambdification of the swing component. You are welcome to apply the patch, but there are numerous changes and they are all automatic. If you have outstanding changes, please coordinate with me and I will re-lambdify to include the changes. Jira: https://javafx-jira.kenai.com/browse/RT-35197 Webrev: See patch is in the JIRA Steve
03-03-2014
SWT lambdified: Changeset: 4742b5a6eb9b Author: snorthov Date: 2014-03-03 15:38 -0500 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/4742b5a6eb9b
03-03-2014
Changeset: a46da8e2262b Author: snorthov Date: 2014-03-03 13:26 -0500 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/a46da8e2262b
03-03-2014
+1
03-03-2014
RT-36000 covers moving those two shipping samples to the samples dir.
28-02-2014
Experiments (3DViewer and Modena) have been lambda-ized. The following files caused compile errors and were reverted: MainController.java OldTestViewer.java TimelineController.java The are can be easily fixed by modifying the source code to move declarations, rename local variables etc. NOTE: 3DViewer and Modena need to move out of experiments and into examples one day soon.
28-02-2014
Changeset: ed4c9c69db41 Author: snorthov Date: 2014-02-28 11:47 -0500 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/ed4c9c69db41
28-02-2014
+1. I did not run every toy but I did run a subset of them and all looks good.
28-02-2014
Hi Lisa, Please review the lambdification of the toys component. You are welcome to apply the patch, but there are numerous changes and they are all automatic. If you have outstanding changes, please coordinate with me and I will re-lambdify to include the changes. Jira: https://javafx-jira.kenai.com/browse/RT-35197 Webrev: See patch is in the JIRA Steve
27-02-2014
Toys are ready to be lambda-ized. The following files caused compile errors and were reverted: HelloFontSize.java HelloListView.java HelloMedia.java HelloModality.java HelloTabPane.java HelloWindowAbuse.java LightMotion.java TestBuilder.java The are can be easily fixed by modifying the source code to move declarations, rename local variables etc.
27-02-2014
Base Lambdification: Changeset: 5f994e8a51cc Author: snorthov Date: 2014-02-27 11:03 -0500 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/5f994e8a51cc
27-02-2014
Yes, that makes sense. Thanks!
27-02-2014
@Martin, We'll keep track of the files in this JIRA for now and revisit, either fixing them by hand or using a fixed IDE. @Kevin, By lambdifying a component at a time, using this JIRA and posting on the OpenJFX list, people should have plenty of time to respond.
27-02-2014
I'm OK with the change. Will you file a new JIRA issue for the files where the automatic lambdification failed?
27-02-2014
IntelliJ have fixed the bugs I filed on lambda conversion: http://youtrack.jetbrains.com/issue/IDEA-120699 http://youtrack.jetbrains.com/issue/IDEA-120698 http://youtrack.jetbrains.com/issue/IDEA-119936 However the latest EA build at http://confluence.jetbrains.com/display/IDEADEV/IDEA+13.1+EAP does not have all the fixes.
26-02-2014
I applied the patch and everything looks fine to me. Using NB 8 RC1 I get no "red squiggles" in the base module, so I'm happy. If Martin is OK with this, and no one has raised any other concerns, I'd say "pull the trigger". A couple of the other modules will need more lead time, since there are multiple people working in those modules. Maybe a day or two "heads-up" as a notice to everyone to get their changes in before the lambdafication. The only thing I noticed is that the auto-lambdifier joins the lines together in a way that often makes for very long lines. For example: modules/base/src/main/java/com/sun/javafx/PlatformUtil.java OLD: javafxPlatform = AccessController.doPrivileged(new PrivilegedAction<String>() { @Override public String run() { return System.getProperty("javafx.platform"); } }); NEW: javafxPlatform = AccessController.doPrivileged((PrivilegedAction<String>) () -> System.getProperty("javafx.platform")); which srolls completely off the right hand screen for me in my IDE. In this case, I would have rather it had left it on the following line, which I see it does for multi-line closures elsewhere. Something like: javafxPlatform = AccessController.doPrivileged((PrivilegedAction<String>) () -> System.getProperty("javafx.platform")); We wouldn't want to hand edit this (of course), but I wonder if this is configurable? As another data point, NB would have done the same thing as IntelliJ did for those lines.
26-02-2014
Hi Martin, Please review the lambdification of the base component. You are welcome to apply the patch, but there are numerous changes and they are all automatic. If you have outstanding changes, please commit them and I will lambdify once more, Jira: https://javafx-jira.kenai.com/browse/RT-35197 Webrev: See patch is in the JIRA Steve
26-02-2014
Base is ready to be lambda-ized. The following files caused compile errors and were reverted: SetBinding.java ListBinding.java ObservableListWrapper.java MapBinding.java ElementObservableListDecorator.java MappingChange.java ObservableSequentialListWrapper.java The are can be easily fixed by moving the declaration fields.
26-02-2014
+1 on the Lambda change for Ensemble8
24-02-2014
I have Lamdified Ensemble8 using the NetBeans lambda tools. These tools caused compile errors in the following files: ColorPickerApp.java ProgressIndicatorApp.java PlaygroundTabs.java SwingInteropApp.java In order to ensure that the lambdification process has no manual changes what so ever, I have reverted the files instead of fixing the compile errors. The fixes are simple, but we want lambdificiation to have no manual steps It is quite likely we will fix these files in another pass. All of the IDE's and the command line compile the code without red squiggles and errors. Note that all of the lambdifiers (Intellij, Edlipse) have problems and produce code that does not compile. I chose the NetBeans tools for Ensemble8 and might choose other lambdifiers in future.
24-02-2014
Some quick measurements indicate that: 1. jfxrt.jar gets about 8% smaller when we rewrite code to use lambdas 2. Start-up time is unchanged. There are some problems with using IntelliJ's automatic conversion to lambdas. I filed a couple of bugs and they are being addressed: http://youtrack.jetbrains.com/issue/IDEA-119936 http://youtrack.jetbrains.com/issue/IDEA-120096
30-01-2014
Nice to see the differences. This is not the complete webrev for all of FX. I suggest you don't do that right away. I suggest we lock down the world, make the changes, then push them out. After this, every patch everyone ever had will be broken.
27-01-2014
http://cr.openjdk.java.net/~dblaukop/webrev-20140128-0018-Lambda/webrev/
27-01-2014
I'm considering either adding RetroLambda as it is or replicating what it does. This is definitely the shortest path to get lambda support in RoboVM.
27-01-2014
Niklas, could RetroLambda be part of the RoboVM build?
07-01-2014
Looks like the gradle plugin for retrolambda is Android-aware: https://github.com/evant/gradle-retrolambda
06-01-2014
This bug (for me) is tracking the issue about using lambda everywhere in FX. We need to decide when and how to do this and do the work right after a milestone. We should run a script that converts all the source. Before we do this, we need to work with the community to find a solution for iOS and Android.
06-01-2014
This doesn't seem like a build issue, unless you mean the part about finding a solution to Android/Dalvik and iOS.
06-01-2014

Relates :	JDK-8087537 - Build scripts should only use JDK_HOME, not JAVA_HOME
Relates :	JDK-8098266 - Restore missing comments that were lost in lambda conversion