RFR(S) 8200746: Remove FlagGuard class

[Erik Helin]

On 05/17/2018 12:06 AM, David Holmes wrote:
> Hi Gerard,
> 
> On 17/05/2018 4:06 AM, Gerard Ziemski wrote:
>> Hi all,
>>
>> Please review this small fix where we remove unused FlagGuard class 
>> and FLAG_GUARD macro from hotspot and gtests.
> 
> Well it seems only unused after you have removed its uses! Why does it 
> turn out not to be needed in test_CollectorPolicy.cpp?

I think it is still needed, I don't think this patch is correct.

When we execute the gtests we execute the TEST_VM tests one-by-one in 
serial order in the same process. This means that if one tests changes a 
global flag, e.g. InitialHeapSize, the next test will see a different 
value for InitialHeapSize. In order to guard the flags, I introduced the 
class FlagGuard (and corresponding helper macro FLAG_GUARD). This means 
that a test, such as test_collectorPolicy.cpp, can easily guard any 
global flags the test intends to mutate, and then the flags will be 
restored to their original values when test has finished.

> Why is the whole test_globals.cpp no longer relevant?

test_globals.cpp currently only tests the FlagGuard class (and 
FLAG_GUARD) macro in globals.hpp, so since Gerard removed the FlagGuard 
class, the tests are no longer relevant. However, see my comment above, 
we can't just remove the FlagGuard class, it is needed!

> The bug report should explain why we don't need these.

Yes, I would also like to see an explanation why this code can be 
removed? To me it seems like it is needed in test_collectorPolicy.cpp, 
but I could be wrong.

Thanks,
Erik

> Thanks,
> David
> 
>> https://bugs.openjdk.java.net/browse/JDK-8200746
>> http://cr.openjdk.java.net/~gziemski/8200746_rev1/
>>
>> Testing: mach5 hs-tier1,2
>>
>>
>> cheers
>>