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

Ulf Zibis Ulf.Zibis at CoSoCo.de
Thu Aug 30 14:03:42 UTC 2012

Am 30.08.2012 08:23, schrieb Joe Darcy:

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 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. Oops, didn't notice that the code is a fastpath in advance.

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. For that reason in project coin I always voted for == semantics as default for switch-on-strings and optionally using case .equals("foo") : syntax for equals() semantics.

But anyway, equals() always first checks ==, and in fail switch-on-strings compares the hashes in 2nd step, so performance should not matter such heavy, compared to the processing which follows, and additionally the following code must not slowly process the, I guess also frequent, case of normal equality. Anyway the following code could be faster, if first upper-casing the the actions string instead comparing each char against upper AND lower case. A // FIXME tag would be nice for that, to not forget the possible tuning in future.

I recommend explicitly mentioning interning "Using identity comparison against known-interned strings for performance benefit." I would additionally prefix with: "First using ..." or "Fastpath, using ..."

-Ulf

More information about the core-libs-dev mailing list