Test for JDK-5108778 Too many instances of java.lang.Boolean created in Java application

Sun Oct 4 07:00:46 UTC 2015

Hi Stuart,

thanks for the feedback. I will answer inline.

On 10/03/2015 04:20 AM, Stuart Marks wrote:
> Hi Sebastian,
>
> Good to see you contributing to OpenJDK again!
>
> I'm not sure it's sensible to have a regression test for this sort of
> thing. This seems more like static code analysis to me. In fact,
> Findbugs already seems to have a pattern that detects calls to the
> Boolean constructor:
>
> http://findbugs.sourceforge.net/bugDescriptions.html#DM_BOOLEAN_CTOR
>
> (I believe that people are running Findbugs over the JDK regularly,
> but we don't really have a workflow for processing diagnostics that it
> generates.)
That would be the best case. It checks the source-code and not the
resulting classfiles. It would be much better if we could integrate
Findbugs somehow.
>
> Now, regression tests are there to prevent bugs from reappearing once
> they're fixed. So how would we do this? For cases like this, there's
> another approach, which is deprecation:
>
> https://bugs.openjdk.java.net/browse/JDK-4941777
>
> If the Boolean constructor is deprecated, then a warning will be
> issued wherever they're called. By default, the JDK build treats
> warnings as errors, so this will effectively prohibit any uses of
> Boolean constructors. (The two legitimate uses of the Boolean
> constructor can be annotated with @SuppressWarnings to prevent this.)
>
> An interesting exercise would be to add the @Deprecated annotation to
> the Boolean constructor and build the JDK and see how many warnings
> result. That might be a quick way to find the code that needs to be
> fixed.
I have tried it, but i think i have done something wrong.
I build via

     make clean JAVAC_WARNINGS="-Xlint:all -Xmaxwarns 10000"
DISABLE_WARNINGS="-Xlint:all" LOG=info images
CONF=linux-x86_64-normal-server-release

And looked at the resulting build.log but get only one warning:

    ~/jdk9$ less build/linux-x86_64-normal-server-release/build.log |
grep warning | grep Boolean

/home/sebastian/jdk9/jaxp/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/JavaClass.java:464:
warning: [deprecation] Boolean(String) in Boolean has been     deprecated

Where the class-scanning shows me 92 hits.

I have looked to some of those to evaluate if there are some
@SuppressWarnings but didn't find any.

So i think the problem is somewhere in the Makefile/Autoconf-files.
Unfortunately i dont't have enough knowledge in those to have an idea
where to search.
>
> As for your other questions:
>
> 2. Boolean is a special case since there are exactly two boxed boolean
> values. Possibly Byte as well, since all Byte values are cached. (See
> spec for Byte.valueOf(byte).) There is a moderate preference for
> valueOf() over the constructors of Character, Integer, and Short,
> since those cache a subset of values. It's not clear to me these are
> worth worrying about though. (Long, Float, and Double don't have caches.)
I agree. Only Byte and maybe Character and Short should be considered. I
have looked at all Wrappertypes and find that also Long has a Cache. I
think Character is worth to think about, because i think in most cases
the cached area is used.

Is there a big downside of using autoboxing/valueOf for all Classes?

>
> 3. I would say that it's more likely that future JDK enhancements
> would be able to optimize auto-boxing than explicit method calls that
> deal with boxed values.
So Integer, Long, Float and Double and maybe Short should be changed not
before we can say what is the best optimization strategy for those.

> 4. Unfortunately, different portions of code are handled by different
> groups, and are thus reviewed on different mailing lists. I think
> asking on jdk9-dev, as you've been doing, about where things need to
> be reviewed, is the right thing to do.
That is no problem. Should we create multiple "sub-task" in JBS for each
area ?
>
> I'll reply on macosx-port-dev on your specific changes there.
>
> s'marks
-- Sebastian
>
>
> On 9/30/15 12:43 PM, Sebastian Sickelmann wrote:
>> Hi,
>>
>> a few days ago i started to investigate JDK-5108778[1] and started
>> discussions
>> for a small parts of it in macosx-port-dev[2] and hotspot-dev[3]. As
>> suggested by
>> Alexandr there should be a test that saves for regression for such
>> changes. I would
>> like to introduce a test like[4], what do you think?
>>
>> It scans for all jimage-files in <java.home>/lib/modules and opens every
>> classfile
>> and scans every-method for a NEW-bytecode to a Wrapper-Type Classname.
>> Every match that is not in the Wrapper-Type itself is reported and
>> counted.
>>
>>
>> I have some questions about this:
>> 1. Is there a good way to get rid of the "9.0" part for reading the
>> classes out of the jimage?
>> 2. What is with other Wrapper-Types (Byte,Short,Integer,Long, Character)
>> is it a good idea to also change such ctor of those? Would someone raise
>> an enhancement
>> for those?
>> 3. How are value-types related to such an issue. Is it counterproductive
>> to change to XYZ.valueOf Method uses, or should we change to autoboxing
>> where possible? I haven't changed to autoboxing where i thought it would
>> be much less readable.
>> 4. Should the changes be discussed in the group-lists? Or is there a
>> good place for discussion off central-changes?
>>
>>
>> -- Sebastian
>>
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-5108778
>> [2]
>> http://mail.openjdk.java.net/pipermail/macosx-port-dev/2015-September/006970.html
>>
>> [3]
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-September/020018.html
>>
>> [4]
>> https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk/jdk-5108778/test_0/webrev/index.html
>>
>>
>