Community
Participate
Working Groups
Verifying I20100817-0800 results, it appears that there was a regression on just after 3.7M1 (since build N20100807-2000) on perf test: - ContentTypeTest#testPluginXMLDirty() This issue looks similar than bug 323009 and bug 323010, but for this test, the regression appears on all machines!?
Will take a look at the test once the setup of the performance tests has been fixed (see bug 323439).
I can reproduce the regression which is around 25%. The regression happened after 3.7 M1 (I20100805-1700). I can see it in N20100806-2000 and also in I20100810-0800. Tracking this down, I found out that the part of the performance test where it fails is not related to Text. The following performance measurement reveals the regression: Reader reader= new StringReader(""); for (int i= 0; i < runs; i++) { IContentType contentType= null; performanceMeter.start(); for (int j= 0; j < iterations; j++) { IContentDescription descriptionFor= Platform.getContentTypeManager().getDescriptionFor(reader, "plugin.xml", new QualifiedName[0]); contentType= descriptionFor.getContentType(); } performanceMeter.stop(); assertEquals(expectedContentType, contentType); } Looking at the core.map file changes between M1 and I20100810-0800 I only found these two changes: plugin@org.eclipse.osgi=v20100809 plugin@org.eclipse.equinox.supplement=v20100809 hence, I tried 3.7 M1 + org.eclipse.osgi from N20100806-2000. This resulted in the regression. Also, comparing profiler results showed that more time got spent in org.eclipse.core.runtime.internal.adaptor.ContextFinder. Moving to Equinox for investigation.
I wonder why ContextFinder even gets used here. ContextFinder is the context class loader. I cannot imagine why performance critical code would use the context class loader in Eclipse.
(In reply to comment #3) > I wonder why ContextFinder even gets used here. ContextFinder is the context > class loader. I cannot imagine why performance critical code would use the > context class loader in Eclipse. Yes, that was my impression too.
Created attachment 179724 [details] Profiler output Note that this profiler output is based on the original test and not the snippet I pasted in comment 2.
Thanks Dani, can you run the profiler against a build that does not show the regression? There was no changes to Context finder in the I20100817-0800 and I cannot think why that would be so much slower in that build.
Created attachment 179740 [details] Data from M1
(In reply to comment #6) > Thanks Dani, can you run the profiler against a build that does not show the > regression? There was no changes to Context finder in the I20100817-0800 and I > cannot think why that would be so much slower in that build. Given the test case I would assume that you can do further traces yourself ;-)
Created attachment 180020 [details] Team Project Set
Since Tom mentioned, he can't see relevant changes in 'org.eclipse.osgi', I retried the scenario (not the simpler one from comment 2 but the one from the original performance test) and got the same result/regression. Here are my detailed steps (Windows XP): 0. close all applications 1. install I20100805-1700 2. install N20100806-2000 3. start using I20100805-1700 with a new workspace 4. install a 1.4 JRE (I took 1.4.2_20) 5. install a 1.5 JRE (I took 1.5.0_22) 6. import the attached Team Project Set (attachment 180020 [details]) 7. import 'org.eclipse.osgi' as binary project (from the active target) 8. open org.eclipse.jdt.text.tests.performance.ContentTypeTest 9. delete all tests except testPluginXMLDirty() 10. select ContentTypeTest.testPluginXMLDirty() in the Outline view 11. Run > Run As > JUnit Plug-in Test ==> at the end it prints information about the test run to the Console 12. remember the Elapsed Process time and repeat step 10 several times to ensure the numbers are stable 13. import 'org.eclipse.osgi' as binary project from N20100806-2000 (choose the 'Directory:' option) 14. Run > Run History > ContentTypeTest.testPluginXMLDirty() ==> at the end it prints information about the test run to the Console 15. remember the Elapsed Process time and repeat step 10 several times to ensure the numbers are stable 16. compare the numbers and see the regression
Hi, Dani. I looked at this in more detail yesterday. I need to investigate some more. It appears this may be related to our change to use jsr14 and, but I need to look into it some more. It looks related to references to XXX.class but I need to do some more testing to confirm that. In ContextFinder.basicFindClassLoaders() we loop over the stack and do check for (stack[i] == ContextFinder.class). If I extract out the ContextFinder.class to be a constant then I see a noticeable performance gain. I am not sure if the move to jsr14 changed the bytecodes in a way to make this less efficient. Right now the ContextFinder.class reference causes a Class.forName each iteration of the loop which is quite expensive.
One possibility is that the loop optimizations are not as good with the jsr14 flag. For example, you could imagine the compiler pulling the ContextFinder.class reference out to a local variable assignment before entering the loop.
> One possibility is that the loop optimizations are not as good with the jsr14 > flag. For example, you could imagine the compiler pulling the > ContextFinder.class reference out to a local variable assignment before > entering the loop. That could well be. We'd have to look at the byte codes to be sure. As a first step I'd simply take the "good" code and compile it with jsr14.
Created attachment 180055 [details] ContextFinder patch (In reply to comment #13) > That could well be. We'd have to look at the byte codes to be sure. As a first > step I'd simply take the "good" code and compile it with jsr14. I did this, it introduced the same performance degradation of at least 25%. I then applied this patch and it brought it back to M1 performance.
I forgot to mention that applying a similar patch to M1 code base did not improve performance so there definitely is bytecode differences causing the slowdown between using jsr14 and our old settings which compiled down to a target of 1.3.
I released the patch. I took a look at the bytecode. It does appear there is an optimization that allows Class.forName call to be done much less when using 1.4 compiler settings vs jsr14. I will leave this bug open until we confirm the performance improvements in a build.
Olivier, maybe we can do something on the compiler side as well.
(In reply to comment #17) > Olivier, maybe we can do something on the compiler side as well. I would assume -target jsr14 would enable the same compiler optimizations as -target 1.3 or 1.4?
(In reply to comment #18) > (In reply to comment #17) > > Olivier, maybe we can do something on the compiler side as well. > > I would assume -target jsr14 would enable the same compiler optimizations as > -target 1.3 or 1.4? Well, jsr14 is not an officially supported options. People using it, are basically on their own, knowing the risks and limitations.
(In reply to comment #16) > I released the patch. I took a look at the bytecode. It does appear there is > an optimization that allows Class.forName call to be done much less when using > 1.4 compiler settings vs jsr14. You mean that when you are comparing the code compiled with 1.4 settings you end up with a better performance than using jsr14? If you would say 1.5 vs jsr14, I could see why. But 1.4 vs jsr14, I need to check exactly what is going on. In 1.5, the class literal is generated using the ldc bytecode, while in 1.4 there is a pretty big code generated for this. Are you sure you are comparing 1.4 vs jsr 1.4 ?
Created attachment 180078 [details] Using compiler settings in org.eclipse.osgi from M1 Here is the bytecode for ContextFinder#basicFindClassLoaders using the project compiler settings from the version of org.eclipse.osgi in M1 Compiler compliance level: 1.4 Generated .class files compatibility: 1.2 Source compatibility: 1.3
Created attachment 180079 [details] Using compiler settings in org.eclipse.osgi post-M1 Here is the same ContextFinder.java file compiled with the project settings from org.eclipse.osgi post-M1. This uses the jsr14 flag
The biggest difference I see is the M1 version contains a line: 51 putstatic org.eclipse.core.runtime.internal.adaptor.ContextFinder.class$0 : java.lang.Class [94] I assume this puts the ContextFinder.class on the stack as a static field that is referenced for the rest of the call instead of calling Class.forName each iteration. I am not fluent in bytecode interpretation so I may be wrong.
(In reply to comment #22) > Here is the same ContextFinder.java file compiled with the project settings > from org.eclipse.osgi post-M1. This uses the jsr14 flag ok, I got it. The value is not cached when jsr14 is used. I'll take care of this. This would be a bug in the compiler in jsr14 mode. Moving to JDT/Core
Created attachment 180084 [details] Proposed fix
Moving to JDT/Core
Created attachment 180086 [details] Proposed fix + regression test
With the fix, the patch for ContextFinder provided in comment 14 is not required anymore.
Released for 3.7M3. Added regression test in: org.eclipse.jdt.core.tests.compiler.regression.Jsr14Test#test1
Verified in N20101003-2000.
