Re: 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