I choose to follow the same pattern as for other flags, although I agree that the extra accessor isn't really needed.
Thanks David! /Staffan On 18 feb 2013, at 05:23, David Holmes <[email protected]> wrote: > Thanks for clarifying this Staffan, I see now how this was setup wrong in the > first place. You are using the right mechanism now. > > I'm not sure UseTLAB warrants its own accessor in VM anymore as the two uses > of it can simply ask for the flag value directly. Your call. > > Thanks, > David > > On 12/02/2013 8:19 PM, Staffan Larsen wrote: >> The values are initialized at VM compile time in the >> VMStructs::localHotSpotVMIntConstants array. I don't think we want to modify >> SA so that values of int constants is made into a dynamic lookup. Or at >> least, that is a fairly significant change to SA. The problem here was to >> think that UseTLAB is a constant when it isn't. >> >> Yes, I thought about a regression test for this, but the valuable test would >> be to verify that no command line flag values are read as constants, and I >> don't see how I could write that. >> >> Thanks, >> /Staffan >> >> On 12 feb 2013, at 01:20, David Holmes <[email protected]> wrote: >> >>> On 10/02/2013 5:06 AM, Staffan Larsen wrote: >>>> Please review this small patch to avoid reading flag values in SA as >>>> constants. Reading them as constants means SA will only see the default >>>> value for these flags. Instead the infrastructure in SA to read command >>>> line flags should be used. In addition the value if EnableInvokeDynamic is >>>> never used, so I removed that from SA. >>> >>> Isn't the real problem in the way db is initialized with the values for >>> these flags? ie shouldn't db.lookupIntConstant("UseTLAB").intValue() be >>> returning the actual value of UseTLAB as it occurs in the VM ? >>> >>> Also we need a regression test for this as obviously it ain't being tested! >>> :( >>> >>> Thanks, >>> David >>> >>>> webrev: http://cr.openjdk.java.net/~sla/8007901/webrev.00/ >>>> bug: http://bugs.sun.com/view_bug.do?bug_id=8007901 (not yet visible) >>>> >>>> Thanks, >>>> /Staffan >>>> >>
