Bug 333678 - byte codes changed in recent builds
Summary: byte codes changed in recent builds
Status: NEW
Alias: None
Product: WTP Releng
Classification: WebTools
Component: releng (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: webtools.releng CLA
QA Contact: Carl Anderson CLA
URL:
Whiteboard:
Keywords:
Depends on: 333732 333822 333825 333827 333828 333829 333830 333831 333832 333833 333834 333835 333836 333837
Blocks:
  Show dependency tree
 
Reported: 2011-01-06 12:54 EST by David Williams CLA
Modified: 2011-09-21 12:38 EDT (History)
2 users (show)

See Also:


Attachments
comparator log of relevent msgs from I build. (7.21 KB, text/plain)
2011-01-06 12:54 EST, David Williams CLA
no flags Details
source code for class (8.32 KB, application/octet-stream)
2011-01-06 13:51 EST, David Williams CLA
no flags Details
decompiled output from previous (3.6) compiler (31.11 KB, text/plain)
2011-01-06 13:52 EST, David Williams CLA
no flags Details
decompiled output from current (3.7M4) compiler (30.54 KB, text/plain)
2011-01-06 13:53 EST, David Williams CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Williams CLA 2011-01-06 12:54:49 EST
Created attachment 186202 [details]
comparator log of relevent msgs from I build.

That is, byte codes changed, even though source didn't, presumably as a result of changing the version of the "base builder". The change is presumably due to a change in either the compiler, the PDE build/settings, or, possibly, due to changes in static constants, etc., in prereq dependencies. This bugzilla will document and track the investigation into the issue ... as well as any actions required. 

This came to light due to our "releng test" which uses p2-provided comparator (during mirror operation), to find cases where a bundle changed contents, even though the version/qualifier of the bundle did not. 

The test failure had apparently been happening for a while (e.g. all week, since we updated base builder, and the releng tests with "new" results, but wasn't noticed till now, due to holidays (and the fact that we are numb to our many JUnit failures). 

http://build.eclipse.org/webtools/committers/wtp-R3.3.0-I/20110106042422/I-3.3.0-20110106042422/

I'll attach the comparator log (since the URL is relatively temporary).
Comment 1 David Williams CLA 2011-01-06 13:04:08 EST
One thing to note/explain is that our I-builds are compared to multiple previous builds ... Starting with M2, then M3, and now, just recently, M4. The comparator log sometimes mentions M2, sometimes M3, sometimes M4. This is probably because in some cases, a plugin has not changed since (the initial) M2, but sometimes has changed since M2, but the I-build version may not have changed since M3 or M4. (remember, if the bundle has changed, there is no comparison done ... which I note just because I get confused myself about what goes on :)  ... if a previous version/qualifier "matches" the current version/qualifier, then that previous version is used in our distribution, not the current one, even if the contents have changed ... so, one urgent issue is if we _need_ the changed content right away (need respin) or if it is an insignificant changed, which could wait ... in all cases, we'll eventually want to tag the bundle so we end up releasing current "changes" ... but, would be best to understand what changed. It'd make a big difference if some underlying constant changed, vs. some optimization that simply removed dead code better, or similar.
Comment 2 David Williams CLA 2011-01-06 13:45:13 EST
I'll describe my "process" here in more detail than usual, since a) I didn't really know how to do is and took a few false turns, thinking I needed a new decompiler or something (but Eclipse's built in version works just fine) and b) it might help others do their own "investigations", if needed.  

To begin with, I decided to focus on this bundle/difference: 

Message 7
canonical: osgi.bundle,org.eclipse.wst.command.env.ui,1.1.3.v201004211805
Difference found for canonical: osgi.bundle,org.eclipse.wst.command.env.ui,1.1.3.v201004211805 between file:/home/data/httpd/download.eclipse.org/webtools/downloads/drops/R3.3.0/S-3.3.0M2-20100923155521/repository/ and file:/shared/webtools/projects/wtp-R3.3.0-I/workdir/I-3.3.0-20110106042422/buildrepository/jst-sdk
The class org/eclipse/wst/command/internal/env/ui/preferences/ActionDialogsPreferencePage.class is different.

Since it is a relatively "low level" bundle, with few dependencies. (relatively speaking). 

According to CVS history, this bundle has not changed since 2006, so makes since that the version/qualifier hasn't changed. 

I retrieved the subject jars from the build machine, being careful to rename to keep them straight ...  I named the old/previous jar with an 'A' suffix, and used 'B' on the currently built version. 

I imported into Eclipse as new (dummy) projects (without source, of course) ... specifically to compare their byte codes. When I did a "compare with each other" then indeed ActionDialogsPreferencePage class showed up as different and was the only difference ... matching the results from the p2 comparator. 

I then opened each of those class files in the JDT's "Class file Viewer" (the normal, built in default, which I never think about :). Each had about 600 lines, and no big differences were obvious by simply looking at the decompiled code, so I copied/pasted the content of that viewer into text files (in Eclipse, add to use ctl-a, ctl-c ... seem to be be no context menus there). 

I think "compare with each other" the two text files, to focus on what was actually different, and it help to focus method-by-method on what what different.

I'll attach files, in case others want to "see" the full story, but in summary, the byte code for 4 methods changed, and in call cases, the new (3.7 based) compiler had removed an unused variable involved in a for-iteration. As one example, 

 private void handleShowAllEvent ()
 {
    Enumeration e = checkBoxes_.elements();
 	for (int i=0; e.hasMoreElements(); i++)
    	{
    		Button dialog = (Button) e.nextElement();
    		dialog.setSelection( false );
    	}                      
 }

Notice the "i" is never used, and does not effect the loop. 

I should note, this bundle is set with a 1.4 bree, and (I'm guessing) could not use any of the "new style" for for-loops. In 1.4, I'm not sure if there is a better way to write such a for loop ... but, the fact that the new compiler generates different byte codes for this case would likely effect lots of code ... but, as far as I can tell, in an insignificant way. (Well, if in fact the generated byte codes are correct, and I assume they are, then in theory it'd be more efficient).
Comment 3 David Williams CLA 2011-01-06 13:51:30 EST
Created attachment 186209 [details]
source code for class
Comment 4 David Williams CLA 2011-01-06 13:52:23 EST
Created attachment 186210 [details]
decompiled output from previous (3.6) compiler
Comment 5 David Williams CLA 2011-01-06 13:53:03 EST
Created attachment 186211 [details]
decompiled output from current (3.7M4) compiler
Comment 6 Olivier Thomann CLA 2011-01-06 15:24:50 EST
This is expected as the local is now reported as unused.
Comment 7 David Williams CLA 2011-01-07 05:02:25 EST
(In reply to comment #6)
> This is expected as the local is now reported as unused.

Thanks Oliver, for the confirmation. I'm going to try and go through each case ... as the 3 or 4 I've gone through are each a little different and want to be sure this one issue really is what accounts for all the differences we are seeing. I'll open 'dependent' bugs for the specific cases, to keep track of the 12 or so cases. 

For (other) interested readers, see JDT bug 328519 for some of of the issues and history of this fix in the compiler. 

I _think_ for WTP, the end result will be that we'll tag and re-release the bundles that have changed byte codes ... but I don't want to do it blindly ... ideally we'll actually remove the 'unused variables' from the source. In some cases, they might even be a coding error that should be fixed by changing the variable name, not just removing it ... but ... we'll have to go through each case to be sure.
Comment 8 David Williams CLA 2011-01-10 02:37:18 EST
I've been through all the cases now ... and only one that is not some (obvious) unused variable case is bug 333828 ... something caused XMLHeadTokenizer byte codes to be different ... but, in general, that is a generated class with lots of case statements so could be some optimization in there?
Comment 9 David Williams CLA 2011-01-10 02:40:45 EST
To be complete, I'll also call out bug 333829 ... it involves an empty switch block ... which I guess means that local variable is not used ... but is obviously "odd code". The previously mentioned XMLHeadTokenizer case is only one that did not have any obvious "odd code".
Comment 10 David Williams CLA 2011-09-21 12:38:26 EDT
mass change back to default assignee and qa contact. I'm not saying I won't work on some :) ... but, won't be all ... so, I think defaults would be best to start over.