RFR (M) 8199282: Remove ValueObj class for allocation subclassing for gc code

[coleen.phillimore at oracle.com]

On 3/13/18 1:44 AM, David Holmes wrote:
> <trimming>
>
> On 13/03/2018 12:23 PM, Kim Barrett wrote:
>>> On Mar 12, 2018, at 5:21 PM, coleen.phillimore at oracle.com wrote:
>>> On 3/12/18 4:51 PM, Kim Barrett wrote:
>>>>> On Mar 12, 2018, at 10:49 AM, coleen.phillimore at oracle.com wrote:
>>>> src/hotspot/share/runtime/osThread.hpp
>>>>
>>>>   56 // I'd make OSThread a ValueObj embedded in Thread to avoid an 
>>>> indirection, but
>>>>    57 // the assembler test in java.cpp expects that it can install 
>>>> the OSThread of
>>>>    58 // the main thread into its own Thread at will.
>>>>
>>>> Removal of the comment doesn't seem right, unless we're really never
>>>> going to change this.  Just deleting "a ValueObj" would be sufficient
>>>> here.
>>>>
>>>> Any idea what assembler test is being referred to here?  I didn't look
>>>> too hard, but I didn't find what prevents embedding OSThread into
>>>> Thread.
>>>
>>> I think the entire comment is obsolete and is something that doesn't 
>>> make sense to change.  No idea what the assembler test is.  Rather 
>>> than trying to reword a now meaningless comment, I removed it.
>
> That comment dates back to 1997 - by John Rose. I believe it refers to 
> this in the ancient vm_main function:
>
>  if (TestAssembler) {
>     printf("Sparc assembler test");
>     MacroAssembler::test();
>     JVM_Exit(0);
>  }
>
> Definitely obsolete and should be removed.

Wow, good sleuthing.
>
>> That just begs the question.  Why isn't OSThread directly embedded in
>> Thread, rather than being a separate object?  Eliminating the
>> indirection seems like it might be useful.  Is there a downside?
>
> We had a project to get rid of OSThread completely a few years ago. 
> Unfortunately it turned out to be somewhat larger to deal with than 
> anticipated and the project was canned due to lack of resources. IIRC 
> the main issues involved resolving OSThread states versus JavaThread 
> states versus java.lang.Thread states.
>
> Given ideally we'd get rid of OSThread do we want to expend effort 
> seeing if we can embed it in Thread? I don't see any real gain here, 
> but as with any change there is a risk of disturbing something 
> non-obvious. If it ain't broke ...

There is still an open RFE: 
https://bugs.openjdk.java.net/browse/JDK-6523731 but it deals with 
thread states.  I guess if there were hot fields in OSThread it would be 
something to look at, and even if there were, it seems like merging 
classes wouldn't be the way to fix that.   The sizeof(Thread) and that 
everything is thrown in Thread/JavaThread argues for a separate class.

If some future work comes along like this, I don't think this comment 
would be helpful, even rewritten to not say ValueObj.

Thanks,
Coleen
>
> David