[14] RFR(s) : 8234173: assert(loader != __null && oopDesc::is_oop(loader)) failed: loader must be oop

[Erik Gahlin]

Looks good.

Erik

On 2020-01-16 05:40, sangheon.kim at oracle.com wrote:
> Hi Kim,
>
> Thanks for reviewing this.
>
> On 1/15/20 5:14 PM, Kim Barrett wrote:
>>> On Jan 15, 2020, at 3:01 PM, sangheon.kim at oracle.com wrote:
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8234173
>>> webrev: http://cr.openjdk.java.net/~sangheki/8234173/webrev.0/
>>> Testing: hs-tier1 ~ 5, jdk-tier1 ~ 3, test/jdk/jdk/jfr and 
>>> test/jdk/jdk/jfr/events/oldobject for 1200 iterations
>>>
>> ------------------------------------------------------------------------------ 
>>
>> src/hotspot/share/jfr/leakprofiler/chains/objectSampleMarker.hpp
>>    70     // now we will "poison" the mark word of the sample object
>>    71     // to marked.
>>
>> I think "poison" is the wrong terminology here now (and probably
>> before too).  Better would be to say we're setting it to "marked" in
>> order to quickly identify sampled objects.
>
> - // now we will "poison" the mark word of the sample object
> - // to marked.
> - // This will be used to quickly identify sample objects
> - // during the reachability search from gc roots.
> + // now we will set the mark word to "marked" in order to quickly
> + // identify sample objects during the reachability search from gc 
> roots.
>
>
>>
>> ------------------------------------------------------------------------------ 
>>
>> src/hotspot/share/oops/markWord.hpp
>>    86 //    [ptr             | 11]  marked             used by 
>> markSweep to mark an object
>>    87 //                                               not valid at 
>> any other time
>>
>> The comment here should be updated.
>
> -// [ptr | 11] marked used by markSweep to mark an object
> -// not valid at any other time
> +// [ptr | 11] marked used to mark an object
>
> As there are more callers of set_marked(), I simplified the comment.
> Or do you have any suggestion?
>
>>
>> ------------------------------------------------------------------------------ 
>>
>>
>> Other than above comment updates, looks good.
>>
> Here's webrev.1 which also updated copyright year.
>
> webrev:
> http://cr.openjdk.java.net/~sangheki/8234173/webrev.1.inc/
> http://cr.openjdk.java.net/~sangheki/8234173/webrev.1/
> Thanks,
> Sangheon
>