Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 343282 - Disable the MethodVerifier class
Summary: Disable the MethodVerifier class
Status: RESOLVED FIXED
Alias: None
Product: JSDT
Classification: WebTools
Component: General (show other bugs)
Version: 3.2.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.2.4   Edit
Assignee: Chris Jaun CLA
QA Contact: Nitin Dahyabhai CLA
URL:
Whiteboard: PMC_approved
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-19 11:41 EDT by Chris Jaun CLA
Modified: 2011-04-28 22:01 EDT (History)
2 users (show)

See Also:
david_williams: pmc_approved+
cmjaun: pmc_approved? (raghunathan.srinivasan)
cmjaun: pmc_approved? (naci.dai)
cmjaun: pmc_approved? (deboer)
cmjaun: pmc_approved? (neil.hauge)
cmjaun: pmc_approved? (kaloyan)
cbridgha: pmc_approved+
thatnitind: review+

Attachments
patch (2.10 KB, patch)
2011-04-19 11:43 EDT, Chris Jaun CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Jaun CLA 2011-04-19 11:41:57 EDT 
Disable calls to the MethodVerifier class because it no longer reports any errors messages that we show.

This should improve performance slightly and remove some potential NPEs.
Comment 1 Chris Jaun CLA 2011-04-19 11:43:01 EDT 
Created attachment 193596 [details]
patch
Comment 2 Chris Jaun CLA 2011-04-19 15:24:38 EDT 
    Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. 

Not necessary to run this code anymore due to changes in semantic validation, so it hurts performance.

    Is there a work-around? If so, why do you believe the work-around is insufficient? 

No

    How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? 

Nothing really to test, just turned off the call to the MethodVerirfier code.

    Give a brief technical overview. Who has reviewed this fix? 

Nitin reviewed the fix. Just commented out the call to MethodVerifier.verify().

    What is the risk associated with this fix? 

None.
Comment 3 David Williams CLA 2011-04-19 16:16:03 EDT 
I'm ok with this, but will ask ... are you sure the method has no side effects, such that if not called, will change some other behavior? (I assume so ... just something I'd check myself, if I had the time). 

Also, in future, as RC weeks pass, please better justify "performance improvements" with some sort of relatively objective numbers ... 1%, 10% 50% of time to open a file (for example) ... just so its easier to see how important it is. 

Thanks for your care,
Comment 4 David Williams CLA 2011-04-21 10:31:17 EDT 
Did this fix go in already, into this week's declared build? If not, then it needs another PMC review/vote. If so, it should be changed to "fixed". 
Thanks
Comment 5 Nitin Dahyabhai CLA 2011-04-21 19:07:32 EDT 
Isn't in 4/21(20) declared build. Requires a one more +1 from a PMC member.  More wouldn't hurt.
Comment 6 Chuck Bridgham CLA 2011-04-22 15:46:32 EDT 
looks good - do we want to remove more calls to this class in the future?
Comment 7 Chris Jaun CLA 2011-04-25 10:23:25 EDT 
Checked into 3.2.4 and HEAD.