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