RFR (XL): 8152664 - Support non-continuous CodeBlobs in HotSpot

[Tobias Hartmann]

Hi Rickard,

I had a look at some parts of the changes. Here are my comments:

codeCache.cpp
-> If we still need NMethodIterator it should be merged with CompiledMethodIterator using C++ templates to avoid code duplication.
sweeper.cpp
-> NMethodMarker is not used and should be removed
-> CompiledMethodMarker differs from NMethodMarker (it should be merged with latest changes)
-> the comment in line 432 is confusing. Shouldn't it be something like "Only flushing nmethod so size..."?
sweeper.hpp
-> the comment describing the sweeper cycle should be updated
-> line 69: "Current nmethod" should be changed to "Current compiled method"
-> is_sweeping(nmethod* which) is not used and can be removed
thread.hpp
-> set_scanned_nmethod() name and comment should be fixed. I also wonder if it's necessary to track/lock CompiledMethod. Shouldn't it be sufficient to lock nmethod?

I also noticed some minor style issues:

codeCache.cpp
-> "CompiledMethod *nm" vs. "CompiledMethod* nm"
codeBlob.hpp
-> typo: "deoptimizatation"
nmethod.hpp
-> wrong indentation in line 265 (whitespace was removed)
vmStructs.cpp
-> unnecessary newline in line 916
-> wrong indentation of "\" at line ends (multiple times)

Best regards,
Tobias


On 07.04.2016 14:12, Rickard Bäckman wrote:
> Hi,
> 
> can I please have review for this patch please?
> 
> So far CodeBlobs have required all the data (metadata, oops, code, etc)
> to be in one continuous blob With this patch we are looking to change
> that. It's been done by changing offsets in CodeBlob to addresses,
> making some methods virtual to allow different behavior and also
> creating a couple of new classes. CompiledMethod now sits inbetween
> CodeBlob and nmethod.
> 
> CR: https://bugs.openjdk.java.net/browse/JDK-8152664
> Webrev: http://cr.openjdk.java.net/~rbackman/8152664/
> 
> Thanks
> /R
>