Request for review: 8000797: NPG: is_pseudo_string_at() doesn't work

[Coleen Phillimore]

On 2/20/2013 3:21 PM, John Rose wrote:
> On Feb 20, 2013, at 11:59 AM, Coleen Phillimore 
> <coleen.phillimore at oracle.com <mailto:coleen.phillimore at oracle.com>> 
> wrote:
>
>> On 2/20/2013 2:51 PM, John Rose wrote:
>>> On Feb 20, 2013, at 10:20 AM, Coleen Phillimore 
>>> <coleen.phillimore at oracle.com <mailto:coleen.phillimore at oracle.com>> 
>>> wrote:
>>>
>>>> Summary: Add JVM_CONSTANT_PseudoString in place of 
>>>> JVM_CONSTANT_Object and use this tag to distinguish patched pseudo 
>>>> strings. The original string is retained if it was present.
>>>
>>> This is reasonable; it is a good cleanup.  If you can propose a name 
>>> better than "PseudoString" I'm all ears.
>>
>> If the string is really meaningless, maybe it can be deleted and we 
>> don't need this JVM_CONSTANT_PseudoString.  The only reason I kept 
>> "String" in the name is because I thought the string would have some 
>> meaning to be preserved.
>
> The string is meaningless.  It is just a waste of UTF8 symbol table space.
>
>>> Consider getting rid of set_has_pseudo_string.  That flag was 
>>> present (IIRC) only to tell the GC that there might be non-perm oops 
>>> in the constant pool.  Do we still need that?
>>
>> I'd be happy to.  I noticed it wasn't being used.   Neither is 
>> _has_invokedynamic for that matter.   _has_preresolution does do 
>> something.
>
> Not any more.  That flag was added for the sake of the 
> internally-generated bytecodes:
> changeset:   2522:ddd894528dbc
> user:        jrose
> date:        Thu Jun 23 17:14:06 2011 -0700
> summary:     7056328: JSR 292 invocation sometimes fails in adapters 
> for types not on boot class path
>
> It appears that we can get rid of all those flags.

Awesome.

>
>>>> I'm not sure how class file reconstitution for pseudo-strings is 
>>>> going to work, but I thought it was prudent to leave the Symbol* in 
>>>> the slot for the patched string.
>>>
>>> If you really wanted to reconstitute a class file for an anonymous 
>>> class, and if that class has oop patching (pseudo-strings), you 
>>> would need either to (a) reconstitute the patches array handed to 
>>> Unsafe.defineAnonymousClass, or (b) accept whatever odd strings were 
>>> there first, as an approximation.  The "odd strings" are totally 
>>> insignificant, and are typically something like 
>>> "CONSTANT_PLACEHOLDER_42" (see 
>>> InvokerBytecodeGenerator::constantPlaceholder).
>>>
>>
>> Maybe there isn't a way or API to reconstitute an anonymous class.   
>> I don't know if there is.
>
> If reconstituting a class means recovering the parameters originally 
> passed to defineClass, then anonymous classes with patching are 
> inherently a special case.  I wouldn't worry about it too much.

Okay, it would probably be done with patching and never recover the 
string constant.
>
>> I'm not sure how to reconstitute a normal class in the first place.   
>> Maybe Serguei can comment.   If this class cannot be reconsitituted, 
>> I'll change this to remove the string in the patched case and won't 
>> need JVM_CONSTANT_PseudoString (and the constant for Object can be 
>> removed too).
>
> Won't we need a tag that says "this thing is a patched constant"? 
>  JVM_CONSTANT_Patched?

If I null out the Symbol* value in JVM_CONSTANT_String, I won't need a 
special constant pool constant.  I missed some places to add 
JVM_CONSTANT_PseudoString in my edit (as you noticed with 
loadable_constant).   I hate this extra constant.

I want to revert to the original change (to null out the Symbol* to 
indicate it's patched), and add the Object tag and flags cleanup.

Thanks for the info how this works.   I'll send out another webrev later.

Coleen

>
> — John

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130220/f8c3c335/attachment-0001.html