RFR: 8217389: JTREG: Clean up, remove unused variable warnings

[Leo Korinth]

Hi!

On 18/01/2019 22:24, Leonid Mesnik wrote:
> Leo
> 
> 1. 
> http://cr.openjdk.java.net/~lkorinth/8217389/00/test/hotspot/jtreg/gc/arguments/TestMaxNewSize.java.udiff.html
> 
> I am not sure how your changes make code clear. Also '128*M' should have 
> spaces.

The main purpose is to remove a compiler warning for unused variable 
(M64). I also do believe 128*M is more obvious at inspection than M128 
(and the construct is used in many places in the code). It also saves 
two lines of code. I agree that 128*M should have spaces, I have seen it 
without spaces with the same type of construct, but now I see it with 
spaces as well. I will add the spaces.

> 
>> On Jan 18, 2019, at 9:23 AM, Leo Korinth <leo.korinth at oracle.com
>> <mailto:leo.korinth at oracle.com>> wrote:
>>
>> Hi,
>>
>> Here I am removing warnings for unused variables.
>>
>> Some variables can not be removed as in the case of creating a new 
>> primitive array without assigning it to a variable. It is not allowed, 
>> in those cases I use @SuppressWarnings("unused").
>>
> 
> I think that it would be better to really use these variables somehow.

Yes, but I am only removing warnings or bugs (or I will never finish this).

> The hotspot compilers might also optimize such arrays initialization 
> since results are not used.  The tests like this
> http://cr.openjdk.java.net/~lkorinth/8217389/00/test/hotspot/jtreg/gc/cslocker/TestCSLocker.java
> just silently pass without any memory allocation and actual testing.
> 
> I think it would be better to change this variables to be public static 
> and/or be used by some external stuff (print zero/last element as 
> example). It guarantee that compiler can't remove this allocation as a 
> dead code.

Yes, I will make them public static variables. I understand that is 
enough to guarantee allocation. I will try to update these test cases 
accordingly. We will then get some actual improvement in tests and not 
only clear warnings :-)

I will prepare a new webrev.

Thanks, Leo

> 
> See following test as example:
> http://hg.openjdk.java.net/jdk/jdk/file/cb7347310fa1/test/hotspot/jtreg/gc/TestMemoryInitialization.java
> 
> Might be some library helper which produce some amount off garbage is 
> useful.
> There are already a lot of methods to produce garbage here but they 
> seems to be overcomplicated. Not sure it make sense to reuse them.
> http://hg.openjdk.java.net/jdk/jdk/file/cb7347310fa1/test/hotspot/jtreg/vmTestbase/nsk/share/gc/gp
> 
> 
> Leonid
> 
>> In some cases the variable is not used at all, and I remove it, but 
>> still add the variable name as a comment after the expression, as the 
>> variable name might have helped as documentation. I also just 
>> commented out "final int CONSTANT_Unicode = 2;" if that information 
>> might be useful in the future, it is a constant that we do not expect 
>> in the test case.
>>
>> Otherwise I just try to remove the code.
>>
>> This fix is somewhat based/dependent on the rest of my "JTREG:" webrevs.
>>
>> Enhancement:
>> https://bugs.openjdk.java.net/browse/JDK-8217389
>>
>> Webrev:
>> http://cr.openjdk.java.net/~lkorinth/8217389/00
>>
>> Testing:
>> passed locally: open/test/hotspot/jtreg/:hotspot_gc
>>
>> Thanks,
>> Leo
>