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

Joe Darcy joe.darcy at oracle.com
Thu Aug 30 06:23:28 UTC 2012

On 8/29/2012 7:07 PM, Stuart Marks wrote:

On 8/29/12 4:56 PM, Joe Darcy wrote: On 8/29/2012 4:20 PM, Stuart Marks wrote: The original code was like this:

427 private static int getMask(String actions) { ... 435 // Check against use of constants (used heavily within the JDK) 436 if (actions == SecurityConstants.FILEREADACTION) { 437 return READ; 438 } else if (actions == SecurityConstants.FILEWRITEACTION) { The various SecurityConstants being used here are Strings. Note that this is doing String comparisons using == which is usually a bug. But this code is relying on String interning to provide object identity for equal string literals, in order to provide a fast path optimization for these common cases. Changing this code to use switch-over-strings would be semantically equivalent, but it would probably slow things down, since switch provides equals() semantics instead of == semantics. This would be a fine point to highlight in the comments around this check! The comment at line 435 did serve to tip me off that something unusual was going on with this code, so it served its purpose. But I'll admit that it's not as explicit as it could be. How about this: // Use object identity comparisons with String constants for performance // reasons (these values are used heavily within the JDK).

I recommend explicitly mentioning interning "Using identity comparison against known-interned strings for performance benefit."

-Joe

More information about the core-libs-dev mailing list