RFR: 8329332: Remove CompiledMethod and CodeBlobLayout classes [v2]

[Stefan Karlsson]

On Wed, 3 Apr 2024 16:38:13 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> src/hotspot/share/code/codeCache.cpp line 1009:
>> 
>>> 1007: int CodeCache::nmethod_count() {
>>> 1008:   int count = 0;
>>> 1009:   for (GrowableArrayIterator<CodeHeap*> heap = _nmethod_heaps->begin(); heap != _nmethod_heaps->end(); ++heap) {
>> 
>> Is there a reason why FOR_ALL_NMETHOD_HEAPS wasn't good fit here? I'm wondering since the similar `CodeCache::blob_count()` still uses one of these macros.
>
> No,  `CodeCache::blob_count()` uses different macro `FOR_ALL_HEAPS(heap)` because it looks for all code blobs, not only nmethods.
> 
> `CodeCache::nmethod_count()` is the only place where `FOR_ALL_NMETHOD_HEAPS ` was used. So I decided to remove the macro.

I didn't say that blob_count used `FOR_ALL_NMETHODS_HEAP`. I wrote "one of these macros". I still think this adds an inconsistency to the code that I don't think is beneficial.

With that said, can't this be written as:

for (CodeHeap* heap : *_nmethod_heaps) {


Maybe yet another opportunity for cleanups.

>> src/hotspot/share/code/nmethod.cpp line 812:
>> 
>>> 810:     // By calling this nmethod entry barrier, it plays along and acts
>>> 811:     // like any other nmethod found on the stack of a thread (fewer surprises).
>>> 812:     nmethod* nm = as_nmethod_or_null();
>> 
>> Calling as_nmethod_or_null() from within functions in the nmethod class is suspicious. Shouldn't all such usages be removed? (I'm fine with doing that as a separate change)
>
> Good catch! The code was moved from CompiledMethod where it made sense but now it is not needed. Here the change I will make:
> 
>      // like any other nmethod found on the stack of a thread (fewer surprises).
> -    nmethod* nm = as_nmethod_or_null();
> -    if (nm != nullptr && bs_nm->is_armed(nm)) {
> +    nmethod* nm = this;
> +    if (bs_nm->is_armed(nm)) {
>        bool alive = bs_nm->nmethod_entry_barrier(nm);

Sounds good.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550216628
PR Review Comment: https://git.openjdk.org/jdk/pull/18554#discussion_r1550217712