RFR JDK-8046282: SA update

[A. Sundararajan]

Hi Poonam,

Looks good to me.

-Sundar

On Monday 16 June 2014 02:45 PM, Poonam Bajaj wrote:
> Hi Sundar,
>
> I have updated the enum classes. The updated webrev is available here:
> http://cr.openjdk.java.net/~poonam/8046282/webrev.01/
>
> Please review the changes and let me know your feedback.
>
> Thanks,
> Poonam
>
>
> On 6/12/2014 1:25 PM, Poonam Bajaj wrote:
>> Hi Sundar,
>>
>> Is it okay with you if I keep the enum and class definitions as they 
>> are now? And can I add you as the reviewer for these changes?
>>
>> Thanks,
>> Poonam
>>
>> On 6/9/2014 2:56 PM, A. Sundararajan wrote:
>>> Since SA is java code, we could have it cleaner..
>>>
>>> my 2 cents,
>>> -Sundar
>>>
>>> On Monday 09 June 2014 02:40 PM, Poonam Bajaj wrote:
>>>> Hi Sundar,
>>>>
>>>> Yes, it is possible to do that. e.g.  G1YCType can be defined like 
>>>> this.
>>>>
>>>> public enum G1YCType {
>>>>   Normal ("Normal"),
>>>>   InitialMark ("Initial Mark"),
>>>>   DuringMark ("During Mark"),
>>>>   Mixed ("Mixed"),
>>>>   G1YCTypeEndSentinel ("Unknown")
>>>>
>>>>   private final String value;
>>>>
>>>>   G1YCType(String val) {
>>>>     this.value = val;
>>>>   }
>>>>   public String value() {
>>>>     return value;
>>>>   }
>>>> }
>>>>
>>>> But, my purpose of having a class and an enum being used in that 
>>>> class was to have the similar kind of code structure on the SA side 
>>>> as is present on the hotspot side. e.g the above is defined as the 
>>>> following on the hotspot side:
>>>>
>>>> 030 enum G1YCType {
>>>> 031   Normal,
>>>> 032   InitialMark,
>>>> 033   DuringMark,
>>>> 034   Mixed,
>>>> 035   G1YCTypeEndSentinel
>>>> 036 };
>>>> 037
>>>> 038 class G1YCTypeHelper {
>>>> 039  public:
>>>> 040   static const char* to_string(G1YCType type) {
>>>> 041     switch(type) {
>>>> 042       case Normal: return "Normal";
>>>> 043       case InitialMark: return "Initial Mark";
>>>> 044       case DuringMark: return "During Mark";
>>>> 045       case Mixed: return "Mixed";
>>>> 046       default: ShouldNotReachHere(); return NULL;
>>>> 047     }
>>>> 048   }
>>>> 049 };
>>>>
>>>> And I have tried to replicate the same on the SA side so that one 
>>>> can easily understand and map the definitions on both the sides.
>>>>
>>>>   27 public class G1YCTypeHelper {
>>>>   28
>>>>   29   public enum G1YCType {
>>>>   30     Normal,
>>>>   31     InitialMark,
>>>>   32     DuringMark,
>>>>   33     Mixed,
>>>>   34     G1YCTypeEndSentinel
>>>>   35   }
>>>>   36
>>>>   37   public static String toString(G1YCType type) {
>>>>   38     switch (type) {
>>>>   39     case Normal:
>>>>   40       return "Normal";
>>>>   41     case InitialMark:
>>>>   42       return "Initial Mark";
>>>>   43     case DuringMark:
>>>>   44       return "During Mark";
>>>>   45     case Mixed:
>>>>   46       return "Mixed";
>>>>   47     default:
>>>>   48       return null;
>>>>   49     }
>>>>   50   }
>>>>   51 }
>>>>
>>>>
>>>> Please let me know if this is still a concern for you.
>>>>
>>>> Thanks,
>>>> Poonam
>>>>
>>>> On 6/9/2014 10:38 AM, A. Sundararajan wrote:
>>>>> The pattern of enum within a class and toString helper to return 
>>>>> string description of it -- is used for many cases.
>>>>>
>>>>> Why not use enums with String accepting constructor and toString 
>>>>> (or new method called "toDescription()) override? You could have 
>>>>> add "Unknown" in these enums to map to an option that is available 
>>>>> in VM -- but not in SA.
>>>>>
>>>>> Since it is a debugger it is expected to work with incomplete 
>>>>> info.. so whereever you map a VM data structure element to java 
>>>>> enum, you can map unknown enum constants to "Unknown" on Java 
>>>>> side.  Of course, if there is a sentinel element defined in VM 
>>>>> side, you could use that itself - no need for "Unknown" in such 
>>>>> cases.
>>>>>
>>>>> It'd nice to simplify that class-within-enum pattern...
>>>>>
>>>>> -Sundar
>>>>>
>>>>> On Saturday 07 June 2014 02:48 PM, Poonam Bajaj wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review these changes for the bug 8046282 for JDK 9. These 
>>>>>> changes add some definitions on the SA side and the supporting 
>>>>>> code on the hotspot side.
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~poonam/8046282/webrev.00/
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8046282
>>>>>>
>>>>>> Thanks,
>>>>>> Poonam
>>>>>>
>>>>>
>>>