RFR[10]: 8180614: Skip range and constraint checks on non-existent flags
Ioi Lam
ioi.lam at oracle.com
Thu May 18 17:42:03 UTC 2017
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.
More information about the hotspot-runtime-dev
mailing list