RFR: 8255231: Avoid upcalls when initializing the statSampler [v3]

[David Holmes]

On Sat, 24 Oct 2020 12:29:05 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Current implementation of the statSampler does upcalls to System.getProperty to collect values for a number of properties that are all provided by the VM itself. And since the sampling starts before any user code run then no property can have changed.
>> 
>> I suggest refactoring the code so that no upcalls are made normally - while asserting this invariant holds using assert-only upcalls. 
>> 
>> This is a small startup optimization - reducing the startup sequence by approx. 300k instructions and 70k branches in my linux-x64 setup.
>
> Claes Redestad has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 19 additional commits since the last revision:
> 
>  - Refactor to remove stable_java_property_counters and clarify comments
>  - Merge branch 'master' into com_ns
>  - Revert unrelated changes to perfData
>  - Merge branch 'master' into com_ns
>  - Improve comments
>  - typo
>  - Missing definition
>  - Extract the shorthand java.version from VersionProps and use it in StatSampler
>  - Improve assert
>  - Assert on missing value
>  - ... and 9 more: https://git.openjdk.java.net/jdk/compare/f77461b0...6e220227

So just to be clear, the statSampler was doing a System.getProperty() upcall, for a property that was previously set by the VM, where the value was initially obtained via a read of the fields in java.lang.VersionProps and stored in the VM in either JDK_Version of VM_version. So now you just use that JDK_Version/VM_Version value directly. And to allow that you had to add in the reading of the "short version" java.version property.

This seems okay in principle. I have a couple of nits:
- can we refer to JDK_Version::java_version instead of "short_version"? I see where the short comes from in VersionProps, but as it represents the java.version value I think we can just use that in the VM for clarity.
- are we concerned that when we store these values in JDK_Version/VM_Version we are artificially constraining their length?
- now that java.version is being read from VersionProps can you please add a comment to that effect in the template:

+ // This field is read by HotSpot
    private static final String java_version =
        "@@VERSION_SHORT@@";

Thanks,
David

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/802