RFR 8071627: Code refactoring to override == operator of Symbol*

[Calvin Cheung]

On 4/8/2015 3:03 PM, Coleen Phillimore wrote:
>
> I've only looked at one part so far.
Thanks for taking a look.
>
> In symbol.hpp   why do you duplicate functions in Symbol* in 
> SymbolRef.  Why not overload operator () and -> like handles do:
We've already overloaded the -> operator as:

   SymbolRef* operator->() {
     return this;
   }

In order to achieve what you've suggested, the -> operator will become:
   Symbol* operator->() {
     return _symbol;
   }

Also the as_uintptr_t() and as_uint() need to be within the Symbol class as:
   uintptr_t as_uintptr_t() {
     return (uintptr_t)this;
   }

   unsigned int as_uint() {
     return (unsigned int)as_uintptr_t();
   }

Then all the wrapper functions in SymbolRef can be removed except for:

int fast_compare(SymbolRef symref) const

that's assuming we don't want to use Symbol* in the code if necessary.

Also, the access to the fast_compare() needs to be changed from -> to .
one example is in instanceKlass.cpp:

     for (int j = 0; j < methods->length() - 1; j++) {
       Method* m1 = methods->at(j);
       Method* m2 = methods->at(j + 1);
       guarantee(m1->name()->fast_compare(m2->name()) <= 0, "methods not 
sorted correctly");
     }
   }

The guarantee statement would have to be changed to:
guarantee(m1->name().fast_compare(m2->name()) <= 0, "methods not sorted 
correctly");

Is is possible.

The main reason we don't want to do that is to avoid exposing the 
_symbol private member if possible.

Calvin

>
>     /* Operators for ease of use */              \
>     type*        operator () () const            { return obj(); } \
>     type*        operator -> () const            { return 
> non_null_obj(); } \
>
>
> Thanks,
> Coleen
>
> On 4/2/15, 8:48 PM, Calvin Cheung wrote:
>> Please review this enhancement for Symbol comparison.
>> This should allow future enhancement such as multiple SymbolTable and 
>> symbols with the same utf8 strings in different tables should be 
>> considered "equivalent".
>>
>> Although this changeset touches many files, the main change is in 
>> src/share/vm/oops/symbol.hpp with a new SymbolRef class. The rest of 
>> the change is mostly replacing Symbol* with SymbolRef.
>>
>> Since currently there's only one single SymbolTable, it isn't 
>> feasible to write a specific testcase for this enhancement. We will 
>> provide a testcase when this enhancement is used.
>>
>> Note also that the copyright header will be fixed before this 
>> changeset is committed.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8071627
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/8071627/webrev.00/
>>
>> Tests:
>>     JPRT
>>     nsk.jvmti on linux_x64
>>
>>     the following performance benmarks on linux_x64, solaris_x64, 
>> solaris_sparc, mac, windows_x64 with no significant performance 
>> degradation:
>>         jetstream, scimark, specjbb2000, specjbb2005, specjvm98, 
>> volano25
>>
>>     some internal class loading performance tests
>>
>> thanks,
>> Calvin
>