RFR: 8159370: Add FlagGuard for easier modification of flags for unit tests

[Erik Helin]

On 2016-06-14, Kim Barrett wrote:
> > On Jun 14, 2016, at 6:58 AM, Erik Helin <erik.helin at oracle.com> wrote:
> > 
> > Hi all,
> > 
> > this patch adds a small utility class that makes it easier for TEST_VM
> > unit tests to safely modify flags. The class FlagGuard will store the
> > value of the flag in its constructor and then restore the flag in its
> > destructor. This is similar IntFlagSetting, UIntFlagSetting etc, but the
> > FlagGuard differs in two aspects:
> > - it does not set a new value
> > - it is "untyped", you can use FlagGuard for flags with any type
> 
> Is there a reason why this is being added to the unit test framework,
> rather than being a generalization of the existing [U]IntFlagSetting
> &etc and placed nearby?  That’s where I would expect to find it.

So my reasoning was that the FlagGuard was pretty slow, since it used
Flag::find_flag, so I didn't want to encourage anyone to use it in
production. Me and Per hacked together a new implementation of
FlagGuard, which uses templates and memcpy instead of Flag::find_flag:

http://cr.openjdk.java.net/~ehelin/8159370/00/

What do you think? I also moved FlagGuard to runtime/globals.hpp and
added some tests. I don't want to update [U]IntFlagSetting since
they also take a new value as parameter, FlagGuard just restores the
value at the end of the scope.

Thanks,
Erik

> > I have also added a macro, GUARD_FLAG, to make it more convenient to
> > guard a flag. A test might now look like:
> > 
> >  TEST_VM(G1Policy, test_calc_max_old_cset_length) {
> >    GUARD_FLAG(MaxGCPauseMillis);
> >    GUARD_FLAG(ParallelGCThreads);
> > 
> >    MaxGCPauseMillis = 500;
> >    ParallelGCThreads = 10;
> >    ASSERT_EQ(10u, G1Policy::calc_max_old_cset_length());
> > 
> >    MaxGCPauseMillis = 50;
> >    ParallelGCThreads = 1;
> >    ASSERT_EQ(2u, G1Policy::calc_max_old_cset_length());
> > 
> >    // All guarded flags are restored here
> >  }
> > 
> > Bug:
> > https://bugs.openjdk.java.net/browse/JDK-8159370
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~ehelin/8159370/00/
> > 
> > Testing:
> > - JPRT
> > 
> > Thanks,
> > Erik
> 
>