RFR: 8217389: JTREG: Clean up, remove unused variable warnings
Leo Korinth
leo.korinth at oracle.com
Mon Jan 21 09:34:06 UTC 2019
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
>
More information about the hotspot-gc-dev
mailing list