RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Claes Redestad
Hi,

analyzing the remaining overhead in command line flag
checking after JDK-8178991, I found that we're taking a
runtime cost scanning for flags that doesn't exist in the
build, e.g., develop flags in a product build.

Since such flags are simply emitted as constants, they
can't[1] be in violation of the constraints, thus I think
we can safely skip such checks altogether.

This removes ~500k instructions from being executed for
no reason during VM startup on a 64-bit JVM.

Bug: https://bugs.openjdk.java.net/browse/JDK-8180614
Webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.00/

Thanks!

/Claes

[1] Unless the default value is invalid, which is a potential
programmer error that would be caught in debug builds
anyhow.
Reply | Threaded
Open this post in threaded view
|

Re: RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Ioi Lam
Hi Claes,


This looks good to me.

As a further optimization, I wonder if we can get rid of all the calls

CommandLineFlagRangeList::check_ranges() -> Flag::find_flag()

This can be done by changing

void emit_range_intx(const char* name, int min, intx max) {
   CommandLineFlagRangeList::add(new CommandLineFlagRange_int(name, min,
max));
}

=>

void emit_range_intx(const char* name, const intx* ptr, int min, intx max) {
   CommandLineFlagRangeList::add(new CommandLineFlagRange_int(name, ptr,
min, max));
}

class CommandLineFlagRange_intx {
   const int* _ptr; ....

- Flag::Error check_int(int value, bool verbose = true) {
+ Flag::Error check_int(bool verbose = true) {
+ int value = *_ptr
       ......
}


Then, the main loop can look like this:

   for (int i=0; i<length(); i++) {
     CommandLineFlagRange* range = at(i);
-   const char* name = range->name();
-   Flag* flag = Flag::find_flag(name, strlen(name), true, true);
-   // We must check for NULL here as lp64_product flags on 32 bit
architecture
-   // can generate range check (despite that they are declared as
constants),
-   // but they will not be returned by Flag::find_flag()
-   if (flag != NULL) {
-     if (flag->is_int()) {
-       int value = flag->get_int();
-       if (range->check_int(value, true) != Flag::SUCCESS) status = false;
+   switch(range->type()) {
+   case CommandLineFlagRange::INT
+       if (range->check_int(true) != Flag::SUCCESS) status = false;

Now, we will do more range checks than before, including flags that
won't be returned by Flag::find_flag, like these guys:

Flag* Flag::find_flag(const char* name, size_t length, bool
allow_locked, bool return_flag) {
   for (Flag* current = &flagTable[0]; current->_name != NULL; current++) {
     if (str_equal(current->_name, current->get_name_length(), name,
length)) {
       ...
       if (!(current->is_unlocked() || current->is_unlocker())) {
         if (!allow_locked) {
           // disable use of locked flags, e.g. diagnostic, experimental,
           // commercial... until they are explicitly unlocked
HERE>>>>  return NULL;


But I think this is better, as we can even find errors in the ergo code
that might have programmatically modified a diagnostic flag, for example.

Or, we can even go one step further, add the ranges into the Flag::flags
table directly. Then, we just need to walk this table sequentially. We
can get rid of all the malloc inside CommandLineFlagRangeList::init()
<-- how many cycles are spent here?

=> try building commandLineFlagRangeList.o with --save-temps and look at
the file commandLineFlagRangeList.ii :-)


And, we can sort Flag::flags alphabetically (by generating this table
using a C program, which includes globals.hpp, during build time). That
way Flag::find_flag can be searched on O(log n) time.

What do you think?


Thanks
- Ioi


On 5/18/17 9:35 AM, Claes Redestad wrote:

> Hi,
>
> analyzing the remaining overhead in command line flag
> checking after JDK-8178991, I found that we're taking a
> runtime cost scanning for flags that doesn't exist in the
> build, e.g., develop flags in a product build.
>
> Since such flags are simply emitted as constants, they
> can't[1] be in violation of the constraints, thus I think
> we can safely skip such checks altogether.
>
> This removes ~500k instructions from being executed for
> no reason during VM startup on a 64-bit JVM.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8180614
> Webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.00/
>
> Thanks!
>
> /Claes
>
> [1] Unless the default value is invalid, which is a potential
> programmer error that would be caught in debug builds
> anyhow.

Reply | Threaded
Open this post in threaded view
|

Re: RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Claes Redestad
Hi Ioi,

On 05/18/2017 07:42 PM, Ioi Lam wrote:
> Hi Claes,
>
>
> This looks good to me.

thanks!

>
> As a further optimization, I wonder if we can get rid of all the calls
>
> CommandLineFlagRangeList::check_ranges() -> Flag::find_flag()

Yes, I was considering approaches to improve this even further as a
follow up, but it turns out it was easier than I had envisioned with
some helpful pointers... ;-)

>
> Now, we will do more range checks than before, including flags that
> won't be returned by Flag::find_flag, like these guys:
>
> Flag* Flag::find_flag(const char* name, size_t length, bool
> allow_locked, bool return_flag) {
>   for (Flag* current = &flagTable[0]; current->_name != NULL;
> current++) {
>     if (str_equal(current->_name, current->get_name_length(), name,
> length)) {
>       ...
>       if (!(current->is_unlocked() || current->is_unlocker())) {
>         if (!allow_locked) {
>           // disable use of locked flags, e.g. diagnostic, experimental,
>           // commercial... until they are explicitly unlocked
> HERE>>>>  return NULL;
>
>
> But I think this is better, as we can even find errors in the ergo
> code that might have programmatically modified a diagnostic flag, for
> example.

Yes, this definitely seems like an improvement.

Updated webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.01/

We discussed offline if all the constraint and range classes could be
turned into templates, and while it superficially seems easy there are
some uses of constraint methods from globals.cpp that needs special
care, so I decided to defer that cleanup.


>
> Or, we can even go one step further, add the ranges into the
> Flag::flags table directly. Then, we just need to walk this table
> sequentially. We can get rid of all the malloc inside
> CommandLineFlagRangeList::init() <-- how many cycles are spent here?
>
> => try building commandLineFlagRangeList.o with --save-temps and look
> at the file commandLineFlagRangeList.ii :-)
>
>
> And, we can sort Flag::flags alphabetically (by generating this table
> using a C program, which includes globals.hpp, during build time).
> That way Flag::find_flag can be searched on O(log n) time.
>
> What do you think?

Might be a lot of effort for no measurable startup gain (since the above
solution gets rid of calls to most calls to find_flag), and other uses
of find_flag doesn't seem very performance critical.

Cleaning up the magic numbers in commandLineFlag*List.cpp would be nice,
though...

Thanks!

/Claes

>
>
> Thanks
> - Ioi
>
>
> On 5/18/17 9:35 AM, Claes Redestad wrote:
>> Hi,
>>
>> analyzing the remaining overhead in command line flag
>> checking after JDK-8178991, I found that we're taking a
>> runtime cost scanning for flags that doesn't exist in the
>> build, e.g., develop flags in a product build.
>>
>> Since such flags are simply emitted as constants, they
>> can't[1] be in violation of the constraints, thus I think
>> we can safely skip such checks altogether.
>>
>> This removes ~500k instructions from being executed for
>> no reason during VM startup on a 64-bit JVM.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180614
>> Webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.00/
>>
>> Thanks!
>>
>> /Claes
>>
>> [1] Unless the default value is invalid, which is a potential
>> programmer error that would be caught in debug builds
>> anyhow.
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Ioi Lam
Hi Claes,

The new webrev looks good. Thanks!

- Ioi

On 5/19/17 4:40 AM, Claes Redestad wrote:

> Hi Ioi,
>
> On 05/18/2017 07:42 PM, Ioi Lam wrote:
>> Hi Claes,
>>
>>
>> This looks good to me.
>
> thanks!
>
>>
>> As a further optimization, I wonder if we can get rid of all the calls
>>
>> CommandLineFlagRangeList::check_ranges() -> Flag::find_flag()
>
> Yes, I was considering approaches to improve this even further as a
> follow up, but it turns out it was easier than I had envisioned with
> some helpful pointers... ;-)
>
>>
>> Now, we will do more range checks than before, including flags that
>> won't be returned by Flag::find_flag, like these guys:
>>
>> Flag* Flag::find_flag(const char* name, size_t length, bool
>> allow_locked, bool return_flag) {
>>   for (Flag* current = &flagTable[0]; current->_name != NULL;
>> current++) {
>>     if (str_equal(current->_name, current->get_name_length(), name,
>> length)) {
>>       ...
>>       if (!(current->is_unlocked() || current->is_unlocker())) {
>>         if (!allow_locked) {
>>           // disable use of locked flags, e.g. diagnostic, experimental,
>>           // commercial... until they are explicitly unlocked
>> HERE>>>>  return NULL;
>>
>>
>> But I think this is better, as we can even find errors in the ergo
>> code that might have programmatically modified a diagnostic flag, for
>> example.
>
> Yes, this definitely seems like an improvement.
>
> Updated webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.01/
>
> We discussed offline if all the constraint and range classes could be
> turned into templates, and while it superficially seems easy there are
> some uses of constraint methods from globals.cpp that needs special
> care, so I decided to defer that cleanup.
>
>
>>
>> Or, we can even go one step further, add the ranges into the
>> Flag::flags table directly. Then, we just need to walk this table
>> sequentially. We can get rid of all the malloc inside
>> CommandLineFlagRangeList::init() <-- how many cycles are spent here?
>>
>> => try building commandLineFlagRangeList.o with --save-temps and look
>> at the file commandLineFlagRangeList.ii :-)
>>
>>
>> And, we can sort Flag::flags alphabetically (by generating this table
>> using a C program, which includes globals.hpp, during build time).
>> That way Flag::find_flag can be searched on O(log n) time.
>>
>> What do you think?
>
> Might be a lot of effort for no measurable startup gain (since the
> above solution gets rid of calls to most calls to find_flag), and
> other uses of find_flag doesn't seem very performance critical.
>
> Cleaning up the magic numbers in commandLineFlag*List.cpp would be
> nice, though...
>
> Thanks!
>
> /Claes
>
>>
>>
>> Thanks
>> - Ioi
>>
>>
>> On 5/18/17 9:35 AM, Claes Redestad wrote:
>>> Hi,
>>>
>>> analyzing the remaining overhead in command line flag
>>> checking after JDK-8178991, I found that we're taking a
>>> runtime cost scanning for flags that doesn't exist in the
>>> build, e.g., develop flags in a product build.
>>>
>>> Since such flags are simply emitted as constants, they
>>> can't[1] be in violation of the constraints, thus I think
>>> we can safely skip such checks altogether.
>>>
>>> This removes ~500k instructions from being executed for
>>> no reason during VM startup on a 64-bit JVM.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180614
>>> Webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.00/
>>>
>>> Thanks!
>>>
>>> /Claes
>>>
>>> [1] Unless the default value is invalid, which is a potential
>>> programmer error that would be caught in debug builds
>>> anyhow.
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Claes Redestad
Hi Ioi,

On 2017-05-19 16:26, Ioi Lam wrote:
> Hi Claes,
>
> The new webrev looks good. Thanks!

Thanks!

For reference this gets down the cost of checking constraints and ranges
from ~3.5M instructions (~1ms) to less than 200k (<0.1ms).

/Claes

>
> - Ioi
>
> On 5/19/17 4:40 AM, Claes Redestad wrote:
>> Hi Ioi,
>>
>> On 05/18/2017 07:42 PM, Ioi Lam wrote:
>>> Hi Claes,
>>>
>>>
>>> This looks good to me.
>>
>> thanks!
>>
>>>
>>> As a further optimization, I wonder if we can get rid of all the calls
>>>
>>> CommandLineFlagRangeList::check_ranges() -> Flag::find_flag()
>>
>> Yes, I was considering approaches to improve this even further as a
>> follow up, but it turns out it was easier than I had envisioned with
>> some helpful pointers... ;-)
>>
>>>
>>> Now, we will do more range checks than before, including flags that
>>> won't be returned by Flag::find_flag, like these guys:
>>>
>>> Flag* Flag::find_flag(const char* name, size_t length, bool
>>> allow_locked, bool return_flag) {
>>>   for (Flag* current = &flagTable[0]; current->_name != NULL;
>>> current++) {
>>>     if (str_equal(current->_name, current->get_name_length(), name,
>>> length)) {
>>>       ...
>>>       if (!(current->is_unlocked() || current->is_unlocker())) {
>>>         if (!allow_locked) {
>>>           // disable use of locked flags, e.g. diagnostic,
>>> experimental,
>>>           // commercial... until they are explicitly unlocked
>>> HERE>>>>  return NULL;
>>>
>>>
>>> But I think this is better, as we can even find errors in the ergo
>>> code that might have programmatically modified a diagnostic flag,
>>> for example.
>>
>> Yes, this definitely seems like an improvement.
>>
>> Updated webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.01/
>>
>> We discussed offline if all the constraint and range classes could be
>> turned into templates, and while it superficially seems easy there
>> are some uses of constraint methods from globals.cpp that needs
>> special care, so I decided to defer that cleanup.
>>
>>
>>>
>>> Or, we can even go one step further, add the ranges into the
>>> Flag::flags table directly. Then, we just need to walk this table
>>> sequentially. We can get rid of all the malloc inside
>>> CommandLineFlagRangeList::init() <-- how many cycles are spent here?
>>>
>>> => try building commandLineFlagRangeList.o with --save-temps and
>>> look at the file commandLineFlagRangeList.ii :-)
>>>
>>>
>>> And, we can sort Flag::flags alphabetically (by generating this
>>> table using a C program, which includes globals.hpp, during build
>>> time). That way Flag::find_flag can be searched on O(log n) time.
>>>
>>> What do you think?
>>
>> Might be a lot of effort for no measurable startup gain (since the
>> above solution gets rid of calls to most calls to find_flag), and
>> other uses of find_flag doesn't seem very performance critical.
>>
>> Cleaning up the magic numbers in commandLineFlag*List.cpp would be
>> nice, though...
>>
>> Thanks!
>>
>> /Claes
>>
>>>
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>> On 5/18/17 9:35 AM, Claes Redestad wrote:
>>>> Hi,
>>>>
>>>> analyzing the remaining overhead in command line flag
>>>> checking after JDK-8178991, I found that we're taking a
>>>> runtime cost scanning for flags that doesn't exist in the
>>>> build, e.g., develop flags in a product build.
>>>>
>>>> Since such flags are simply emitted as constants, they
>>>> can't[1] be in violation of the constraints, thus I think
>>>> we can safely skip such checks altogether.
>>>>
>>>> This removes ~500k instructions from being executed for
>>>> no reason during VM startup on a 64-bit JVM.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180614
>>>> Webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.00/
>>>>
>>>> Thanks!
>>>>
>>>> /Claes
>>>>
>>>> [1] Unless the default value is invalid, which is a potential
>>>> programmer error that would be caught in debug builds
>>>> anyhow.
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Gerard Ziemski
Thank you Claes and Ioi for 2 very nice optimizations!

Looks good!

ps. Claes, did you run "hotspot/test/runtime/CommandLine/OptionsValidation” with this change?


cheers

> On May 19, 2017, at 9:44 AM, Claes Redestad <[hidden email]> wrote:
>
> Hi Ioi,
>
> On 2017-05-19 16:26, Ioi Lam wrote:
>> Hi Claes,
>>
>> The new webrev looks good. Thanks!
>
> Thanks!
>
> For reference this gets down the cost of checking constraints and ranges from ~3.5M instructions (~1ms) to less than 200k (<0.1ms).
>
> /Claes
>
>>
>> - Ioi
>>
>> On 5/19/17 4:40 AM, Claes Redestad wrote:
>>> Hi Ioi,
>>>
>>> On 05/18/2017 07:42 PM, Ioi Lam wrote:
>>>> Hi Claes,
>>>>
>>>>
>>>> This looks good to me.
>>>
>>> thanks!
>>>
>>>>
>>>> As a further optimization, I wonder if we can get rid of all the calls
>>>>
>>>> CommandLineFlagRangeList::check_ranges() -> Flag::find_flag()
>>>
>>> Yes, I was considering approaches to improve this even further as a follow up, but it turns out it was easier than I had envisioned with some helpful pointers... ;-)
>>>
>>>>
>>>> Now, we will do more range checks than before, including flags that won't be returned by Flag::find_flag, like these guys:
>>>>
>>>> Flag* Flag::find_flag(const char* name, size_t length, bool allow_locked, bool return_flag) {
>>>>  for (Flag* current = &flagTable[0]; current->_name != NULL; current++) {
>>>>    if (str_equal(current->_name, current->get_name_length(), name, length)) {
>>>>      ...
>>>>      if (!(current->is_unlocked() || current->is_unlocker())) {
>>>>        if (!allow_locked) {
>>>>          // disable use of locked flags, e.g. diagnostic, experimental,
>>>>          // commercial... until they are explicitly unlocked
>>>> HERE>>>>  return NULL;
>>>>
>>>>
>>>> But I think this is better, as we can even find errors in the ergo code that might have programmatically modified a diagnostic flag, for example.
>>>
>>> Yes, this definitely seems like an improvement.
>>>
>>> Updated webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.01/
>>>
>>> We discussed offline if all the constraint and range classes could be turned into templates, and while it superficially seems easy there are some uses of constraint methods from globals.cpp that needs special care, so I decided to defer that cleanup.
>>>
>>>
>>>>
>>>> Or, we can even go one step further, add the ranges into the Flag::flags table directly. Then, we just need to walk this table sequentially. We can get rid of all the malloc inside CommandLineFlagRangeList::init() <-- how many cycles are spent here?
>>>>
>>>> => try building commandLineFlagRangeList.o with --save-temps and look at the file commandLineFlagRangeList.ii :-)
>>>>
>>>>
>>>> And, we can sort Flag::flags alphabetically (by generating this table using a C program, which includes globals.hpp, during build time). That way Flag::find_flag can be searched on O(log n) time.
>>>>
>>>> What do you think?
>>>
>>> Might be a lot of effort for no measurable startup gain (since the above solution gets rid of calls to most calls to find_flag), and other uses of find_flag doesn't seem very performance critical.
>>>
>>> Cleaning up the magic numbers in commandLineFlag*List.cpp would be nice, though...
>>>
>>> Thanks!
>>>
>>> /Claes
>>>
>>>>
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>>
>>>> On 5/18/17 9:35 AM, Claes Redestad wrote:
>>>>> Hi,
>>>>>
>>>>> analyzing the remaining overhead in command line flag
>>>>> checking after JDK-8178991, I found that we're taking a
>>>>> runtime cost scanning for flags that doesn't exist in the
>>>>> build, e.g., develop flags in a product build.
>>>>>
>>>>> Since such flags are simply emitted as constants, they
>>>>> can't[1] be in violation of the constraints, thus I think
>>>>> we can safely skip such checks altogether.
>>>>>
>>>>> This removes ~500k instructions from being executed for
>>>>> no reason during VM startup on a 64-bit JVM.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180614
>>>>> Webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.00/
>>>>>
>>>>> Thanks!
>>>>>
>>>>> /Claes
>>>>>
>>>>> [1] Unless the default value is invalid, which is a potential
>>>>> programmer error that would be caught in debug builds
>>>>> anyhow.
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Claes Redestad
Hi Gerard,

On 2017-05-19 18:07, Gerard Ziemski wrote:
> Thank you Claes and Ioi for 2 very nice optimizations!
>
> Looks good!

Thanks!

>
> ps. Claes, did you run "hotspot/test/runtime/CommandLine/OptionsValidation” with this change?

Yes - all pass.

/Claes

>
>
> cheers
>
>> On May 19, 2017, at 9:44 AM, Claes Redestad <[hidden email]> wrote:
>>
>> Hi Ioi,
>>
>> On 2017-05-19 16:26, Ioi Lam wrote:
>>> Hi Claes,
>>>
>>> The new webrev looks good. Thanks!
>> Thanks!
>>
>> For reference this gets down the cost of checking constraints and ranges from ~3.5M instructions (~1ms) to less than 200k (<0.1ms).
>>
>> /Claes
>>
>>> - Ioi
>>>
>>> On 5/19/17 4:40 AM, Claes Redestad wrote:
>>>> Hi Ioi,
>>>>
>>>> On 05/18/2017 07:42 PM, Ioi Lam wrote:
>>>>> Hi Claes,
>>>>>
>>>>>
>>>>> This looks good to me.
>>>> thanks!
>>>>
>>>>> As a further optimization, I wonder if we can get rid of all the calls
>>>>>
>>>>> CommandLineFlagRangeList::check_ranges() -> Flag::find_flag()
>>>> Yes, I was considering approaches to improve this even further as a follow up, but it turns out it was easier than I had envisioned with some helpful pointers... ;-)
>>>>
>>>>> Now, we will do more range checks than before, including flags that won't be returned by Flag::find_flag, like these guys:
>>>>>
>>>>> Flag* Flag::find_flag(const char* name, size_t length, bool allow_locked, bool return_flag) {
>>>>>   for (Flag* current = &flagTable[0]; current->_name != NULL; current++) {
>>>>>     if (str_equal(current->_name, current->get_name_length(), name, length)) {
>>>>>       ...
>>>>>       if (!(current->is_unlocked() || current->is_unlocker())) {
>>>>>         if (!allow_locked) {
>>>>>           // disable use of locked flags, e.g. diagnostic, experimental,
>>>>>           // commercial... until they are explicitly unlocked
>>>>> HERE>>>>  return NULL;
>>>>>
>>>>>
>>>>> But I think this is better, as we can even find errors in the ergo code that might have programmatically modified a diagnostic flag, for example.
>>>> Yes, this definitely seems like an improvement.
>>>>
>>>> Updated webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.01/
>>>>
>>>> We discussed offline if all the constraint and range classes could be turned into templates, and while it superficially seems easy there are some uses of constraint methods from globals.cpp that needs special care, so I decided to defer that cleanup.
>>>>
>>>>
>>>>> Or, we can even go one step further, add the ranges into the Flag::flags table directly. Then, we just need to walk this table sequentially. We can get rid of all the malloc inside CommandLineFlagRangeList::init() <-- how many cycles are spent here?
>>>>>
>>>>> => try building commandLineFlagRangeList.o with --save-temps and look at the file commandLineFlagRangeList.ii :-)
>>>>>
>>>>>
>>>>> And, we can sort Flag::flags alphabetically (by generating this table using a C program, which includes globals.hpp, during build time). That way Flag::find_flag can be searched on O(log n) time.
>>>>>
>>>>> What do you think?
>>>> Might be a lot of effort for no measurable startup gain (since the above solution gets rid of calls to most calls to find_flag), and other uses of find_flag doesn't seem very performance critical.
>>>>
>>>> Cleaning up the magic numbers in commandLineFlag*List.cpp would be nice, though...
>>>>
>>>> Thanks!
>>>>
>>>> /Claes
>>>>
>>>>>
>>>>> Thanks
>>>>> - Ioi
>>>>>
>>>>>
>>>>> On 5/18/17 9:35 AM, Claes Redestad wrote:
>>>>>> Hi,
>>>>>>
>>>>>> analyzing the remaining overhead in command line flag
>>>>>> checking after JDK-8178991, I found that we're taking a
>>>>>> runtime cost scanning for flags that doesn't exist in the
>>>>>> build, e.g., develop flags in a product build.
>>>>>>
>>>>>> Since such flags are simply emitted as constants, they
>>>>>> can't[1] be in violation of the constraints, thus I think
>>>>>> we can safely skip such checks altogether.
>>>>>>
>>>>>> This removes ~500k instructions from being executed for
>>>>>> no reason during VM startup on a 64-bit JVM.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180614
>>>>>> Webrev: http://cr.openjdk.java.net/~redestad/8180614/hotspot.00/
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> /Claes
>>>>>>
>>>>>> [1] Unless the default value is invalid, which is a potential
>>>>>> programmer error that would be caught in debug builds
>>>>>> anyhow.

Reply | Threaded
Open this post in threaded view
|

Re: RFR[10]: 8180614: Skip range and constraint checks on non-existent flags

Claes Redestad
Hi again,

On 05/21/2017 11:08 PM, Claes Redestad wrote:
>> ps. Claes, did you run
>> "hotspot/test/runtime/CommandLine/OptionsValidation” with this change?
>
> Yes - all pass.

I did find another tests that failed, though:
hotspot/test/runtime/CompressedOops/ObjectAlignment.java

Turns out this was due to me incorrectly checking #ifdef LP64 instead of
#ifdef _LP64

I've updated the webrev in place with this typo/bug fixed.

Thanks!

/Claes