[11u] RFR(M): 8219586 backport: CodeHeap State Analytics processes dead nmethods

Mon Apr 19 12:21:17 UTC 2021

Hi Paul, 

thank you for waiting. The test results are in. In summary: no issues which could be related to the pending downport changes in any way.

For some more details, you may refer to the attached PDF. It gives you an idea of what tests we run every night. Green squares indicate all subtests of the suite completed OK. Purple squares indicate "test still running" and the yellow squares show suites with at least one subtest failing. I checked all yellow squares. Each represents just one failing subtest, and all failures are completely unrelated to the backport.

There were no builds and tests last night for rs6000_64 (AIX), sun_64 (Solaris/SPARC), and darwinintel64 (MacOS/Intel). Their trigger time was before I had the patches ready and in the queue. As this is all platform-independent code, I do not expect we missed any findings.

The comments in codeHeapState.cpp are correct. I couldn't find a reference to PrintCodeHeapAnalytics or similar.

I believe the patch for 8217465 is incomplete because it does not delete the macro definitions for STRINGSTREAM_FLUSH and STRINGSTREAM_FLUSH_LOCKED.

Thank you again for the effort you put into this downport.
Lutz

On 16.04.21, 19:28, "Hohensee, Paul" <hohensee at amazon.com> wrote:

    Forgot to write I agree with you on not including 8214526.

    -----Original Message-----
    From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> on behalf of "Hohensee, Paul" <hohensee at amazon.com>
    Date: Friday, April 16, 2021 at 10:03 AM
    To: "Schmidt, Lutz" <lutz.schmidt at sap.com>
    Cc: "jdk-updates-dev at openjdk.java.net" <jdk-updates-dev at openjdk.java.net>
    Subject: RE: [11u] RFR(M): 8219586 backport: CodeHeap State Analytics processes dead nmethods

    If we meet up again in Belgium, I'll take you up on the beer offer. :)

    I see why I thought 8214526 was included: it's in a comment in your webrev for codeHeapState.cpp. I guess the comment should be changed to use -Xlog, but I don't know exactly how it should be worded.

    -----Original Message-----
    From: "Schmidt, Lutz" <lutz.schmidt at sap.com>
    Date: Friday, April 16, 2021 at 9:48 AM
    To: "Hohensee, Paul" <hohensee at amazon.com>
    Cc: "jdk-updates-dev at openjdk.java.net" <jdk-updates-dev at openjdk.java.net>
    Subject: RE: [11u] RFR(M): 8219586 backport: CodeHeap State Analytics processes dead nmethods

    Hi Paul,

    sorry for the mess! I owe you a beer!

    8214526:
    This change is not in my downport change. It should not be downported to 11u because it changes how you control CodeHeapAnalytics. The sole purpose was to switch from -Xlog:codecache=<level> to -XX:+PrintCodeHeapAnalytics. There was a long and fierce discussion about how to control the feature. Print* is deprecated and -Xlog does not fit the requirements. I don't want to revive that.

    8219586:
    The differences in lock handling are caused by major changes in mutex.cpp/hpp. I found no way to write the locks which works for both, jdk11u and tip.

    The issues that were found surfaced during option stress testing. These are Oracle-internal tests where they exercise the command-line options. At least that's my understanding. Other than that, I do not know of specific tests. I conducted manual tests only. It's anyway hard to write an automated test which performs meaningful checks without being very shaky.

    I will take your webrevs (except 8214526) and feed them into our build and test farm over the weekend. If everything runs fine, I should have results by Monday morning. I will let you know immediately.

    Enjoy your weekend!
    Lutz

    On 16.04.21, 18:06, "Hohensee, Paul" <hohensee at amazon.com> wrote:

        The maintainers prefer to see individual backports rather than bulk ones.

        You're bulk backporting codeHeapState.cpp. Instead of that, you could backport these commits individually.

        8219586: CodeHeap State Analytics processes dead nmethods
        8217465: [REDO] - Optimize CodeHeap Analytics
        8216314: SIGILL in CodeHeapState::print_names()
        8214526: Change CodeHeap State Analytics control from UL to Print*

        8214526: applies cleanly.
        8216314: needs a copyright change in codeHeapState.cpp.
        8217465:
        Needs an implicit bufferedStream copy constructor removed in codeHeapState.cpp. This change was in 8224487, which was previously backported to 11u, but the change was omitted in that backport.
        8219586:
        Context adjustment in codeHeapState.cpp because 8183574 has not been backported.
        Context adjustment in compiledMethod.cpp. Also, your webrev has global_lock_1 and function_lock_1 used with _no_safepoint_check_flag, but the tip version uses _safepoint_check_flag.
        Context adjustment in mutexLocker.cpp.

        I made incremental webrevs of the above. They apply in sequence.

        https://cr.openjdk.java.net/~phh/8214526/webrev.11u.01/
        https://cr.openjdk.java.net/~phh/8216314/webrev.11u.01/
        https://cr.openjdk.java.net/~phh/8217465/webrev.11u.01/
        https://cr.openjdk.java.net/~phh/8219586/webrev.11u.02/

        I'm running tier1 on linux-x64, but don't have access to other platforms. Would you be able to apply these patches and test them thoroughly? Are there any specific tests I can run beyond tier1?

        Thanks,
        Paul

        -----Original Message-----
        From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> on behalf of "Schmidt, Lutz" <lutz.schmidt at sap.com>
        Date: Wednesday, April 14, 2021 at 9:09 AM
        To: "jdk-updates-dev at openjdk.java.net" <jdk-updates-dev at openjdk.java.net>
        Subject: [11u] RFR(M): 8219586 backport: CodeHeap State Analytics processes dead nmethods

        Dear Community,

        I would happily accept reviews for this downport change. The change puts an end to discussions on the "accessing dead storage" topic, at least to the best of Erik Österlund's and my knowledge.

        Most merge conflicts (and the complicated ones) were encountered codeHeapState.{c|h}pp. To resolve them, I copied the files from OpenJDK/jdk. That was the desired state for these files anyway.

        To resolve compileBroker.cpp, I copied the OpenJDK/jdk version of CompileBroker::print_heapinfo() and retrofitted the Monitor and Mutex objects and calls to the jdk11 interfaces.

        The remaining conflicts were trivial to resolve.

        Original bug:          https://bugs.openjdk.java.net/browse/JDK-8219586
        Downport webrev:       https://cr.openjdk.java.net/~lucy/webrevs/8219586.11u.01/
        Merge conflicts:       https://cr.openjdk.java.net/~lucy/webrevs/8219586-jdk11u.conflicts
        Conflict resolve diff: https://cr.openjdk.java.net/~lucy/webrevs/8219586-jdk11u.conflictresolve

        Tests:
        SAP's internal build and test farm (all OpenJDK platforms (no 32-bit), and more). Tests include JCK, jtreg (hotspot and jdk), and SAP-private tests. Results pending (expected by April 15th, start of business).

        Thanks,
        Lutz