RFR(S): 8250635: MethodArityHistogram should use Compile_lock in favour of fancy checks

[Schmidt, Lutz]

Hi Martin, 

good question. 

Originally, the iteration was only protected by the CodeCache_lock. That proved insufficient: the CodeCache_lock only protects against structural changes in the CodeCache. The contents of the individual code blobs can be, and is, modified independently. 

By acquiring the Compile_lock, those modifications are blocked while iterating. 

With the help of a consistency check (not contained in the RFR code), it was found that there is a slight chance to see the case (nm != NULL) && (method() == NULL). That chance is eliminated by adding the is_alive() check which is less invasive compared to adding a new nmethods_do() variant. 

Regards,
Lutz

On 28.08.20, 17:02, "Doerr, Martin" <martin.doerr at sap.com> wrote:

    Hi Lutz,

    just for my understanding: What exactly are we protecting against by holding Compile_lock?
    Is it for concurrent initialization or concurrent unloading?

    Note that it's also possible to iterate only over alive nmethods:
    NMethodIterator iter(NMethodIterator::only_alive);

    Best regards,
    Martin


    > -----Original Message-----
    > From: hotspot-compiler-dev <hotspot-compiler-dev-
    > retn at openjdk.java.net> On Behalf Of Schmidt, Lutz
    > Sent: Mittwoch, 26. August 2020 17:19
    > To: hotspot-compiler-dev at openjdk.java.net
    > Subject: [CAUTION] RFR(S): 8250635: MethodArityHistogram should use
    > Compile_lock in favour of fancy checks
    > 
    > Dear all,
    > 
    > may I please request reviews for this small enhancement? Instead of calling a
    > method doing complicated and fancy (hard to understand) checks, the
    > iteration over all nmethods is now protected by holding the Compile_lock in
    > addition to the CodeCache_lock.
    > 
    > Bug:    https://bugs.openjdk.java.net/browse/JDK-8250635
    > Webrev: https://cr.openjdk.java.net/~lucy/webrevs/8250635.00/
    > 
    > Thank you!
    > Lutz
    >