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

Christian Thalinger christian.thalinger at oracle.com
Fri Apr 8 20:40:52 UTC 2016


> On Apr 7, 2016, at 4:50 AM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
> 
> 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”

+ CompiledMethod* CodeCache::find_compiled(void* start) {
+   CodeBlob *cb = find_blob(start);
+   assert(cb == NULL || cb->is_compiled(), "did not find an compiled_method”);
Should be “a CompiledMethod”.

> 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)

compileBroker.cpp:
!     CompiledMethod* code = method->code();
+     if (code == NULL) {
+       return (nmethod*) code;
Return NULL instead of casting.

> 
> 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
>> 



More information about the hotspot-dev mailing list