Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

[Stuart Marks]

On 8/27/12 3:55 AM, Doug Lea wrote:
> The underlying issue is that code size is one of the criteria
> that JITs use to decide to compile/inline etc. So long as they do
> so, there will be cases here and there where it critically
> important to keep sizes small in bottleneck code. Not many,
> but still enough for me to object to efforts that might
> blindly increase code size for the sake of warnings cleanup.

I'm pleased that warnings cleanup has attracted this much attention. :-)

I was wondering where rule about @SuppressWarnings and local variables 
originally came from, and I tracked this back to Effective Java, Item 24, which 
says (as part of a fairly long discussion)

     If you find yourself using the SuppressWarnings annotation
     on a method or constructor that's more than one line long,
     you may be able to move it onto a local variable declaration.
     You may have to declare a new local variable, but it's worth it.

Aha! So it's all Josh Bloch's fault! :-)

In the warnings cleanup thus far, and in Dan's webrev, we've followed this rule 
fairly strictly. But since we seem to have evidence that this change isn't 
worth it, we should consider relaxing the rule for performance-critical code. 
How about adding a local variable for @SuppressWarnings only if the method is, 
say, longer than ten lines? (Or some other suitable threshold.) For short 
methods the annotation should be placed on the method itself.

The risk of suppressing other warnings inadvertently is pretty small in a short 
method, and short methods are the ones most likely to be impacted by the 
addition of a local variable affecting compilation/inlining decisions. Right?

(Also, while I've recommended that people follow the local variable rule fairly 
strictly, I think it tends to garbage up short methods. So I wouldn't mind 
seeing the rule relaxed on readability grounds as well.)

s'marks