eclipse-openj9 / openj9-utils

Other
16 stars 29 forks source link

Added MonitorContendedEnter event #40

Closed yathamravali closed 3 years ago

yathamravali commented 3 years ago

Prints the current and owner thread details i.e threadname threadId and nativethreadId in MonitorContendedEnter Event

Fixes: #20 Fixes: #38 Signed-off-by: Ravali Yatham rayatha1@in.ibm.com

yathamravali commented 3 years ago

@mpirvu - Please review

yathamravali commented 3 years ago

According to the documentation, you must add can_get_monitor_info as a capability. Also, I see some json arrays being created and I guess that the intent was to capture the backtrace, but there is no JVMTI calls to GetStackTrace. How does the output look with this code?

Sample output:

{
  "body": {
    "monitorContendedEnterEvent": [
      {
        "CurrentThread": [
          {
            "threadID": 35,
            "threadName": "Thread-14",
            "threadNativeID": 52205
          }
        ],
        "OwnerThread": [
          {
            "threadID": 27,
            "threadName": "Thread-6",
            "threadNativeID": 52197
          }
        ]
      }
    ]
  },
  "from": "Server",
  "timestamp": 1614502763039258708
},
yathamravali commented 3 years ago

I can still see indentation issues, Trying to fix that

mpirvu commented 3 years ago

Under "CurrentThread" label there is an array which has only one entry:

  {
            "threadID": 35,
            "threadName": "Thread-14",
            "threadNativeID": 52205
   }

Similarly for "OwnerThread". There is no need for an array. The array would be needed for the stacktraces of those threads. Code for monitorContendedEnteredEvent should be relevant in this sense.

yathamravali commented 3 years ago

@mpirvu - Agree with you. I've raised another commit wherein removed the use of arrays and I'm storing it in objects. Output looks as below

{
  "body": {
    "CurrentThread": {
      "threadID": 36,
      "threadName": "Thread-15",
      "threadNativeID": 144831
    },
    "OwnerThread": {
      "threadID": 28,
      "threadName": "Thread-7",
      "threadNativeID": 144823
    },
    "eventType": "MonitorContendedEnterEvent"
  },
  "from": "Server",
  "timestamp": 1614685264037736438
}
mpirvu commented 3 years ago

"eventType": "MonitorContendedEnterEvent" is going to be handled by https://github.com/eclipse/openj9-utils/pull/37

mpirvu commented 3 years ago

It's debatable whether we want the stack trace and the type of the object current thread is waiting on. I say it's debatable because that information can be had from the "MonitorContendedEnteredEvent". However, we do need the stack trace of the thread that owns the monitor.

yathamravali commented 3 years ago

It's debatable whether we want the stack trace and the type of the object current thread is waiting on. I say it's debatable because that information can be had from the "MonitorContendedEnteredEvent". However, we do need the stack trace of the thread that owns the monitor.

ok, Can we do that in another PR? As this PR focuses on issue #20

In addition, Can you check if indentation looks good or not with latest commit?

yathamravali commented 3 years ago

@mpirvu - Addressed review comments. Sample output for MonitorContendedEnter looks like below

{
  "body": {
    "CurrentThread": {
      "stackTrace": [
        {
          "class": "LMyThread;",
          "method": "foo",
          "signature": "()V"
        },
        {
          "class": "LMyThread;",
          "method": "run",
          "signature": "()V"
        }
      ],
      "threadID": 33,
      "threadName": "Thread-12",
      "threadNativeID": 425709
    },
    "OwnerThread": {
      "stackTrace": [
        {
          "class": "LWork;",
          "method": "work",
          "signature": "(I)V"
        },
        {
          "class": "LMyThread;",
          "method": "foo",
          "signature": "()V"
        },
        {
          "class": "LMyThread;",
          "method": "run",
          "signature": "()V"
        }
      ],
      "threadID": 44,
      "threadName": "Thread-23",
      "threadNativeID": 425720
    },
    "class": "Work",
    "monitorHash": "0x75a59030",
    "numTypeContentions": 253
  },
  "from": "Server",
  "timestamp": 1615284145152842865
},
mpirvu commented 3 years ago

Felix is asking about an ETA for this feature.

yathamravali commented 3 years ago

Felix is asking about an ETA for this feature.

Will try to complete it by tomorrow EOB

yathamravali commented 3 years ago

@mpirvu - PTAL

mpirvu commented 3 years ago

Before merging, I would like conformation that the code has been tested and is working as intended.

yathamravali commented 3 years ago

Yes tested, the sample output for monitorContendedEnter looks like

{
  "body": {
    "CurrentThread": {
      "stackTrace": [
        {
          "class": "LMyThread;",
          "method": "foo",
          "signature": "()V"
        }
      ],
      "threadID": 37,
      "threadName": "Thread-16",
      "threadNativeID": 43510
    },
    "OwnerThread": {
      "stackTrace": [
        {
          "class": "LWork;",
          "method": "work",
          "signature": "(I)V"
        }
      ],
      "threadID": 42,
      "threadName": "Thread-21",
      "threadNativeID": 43515
    },
    "class": "Work",
    "monitorHash": "0x9532eb5a",
    "numTypeContentions": 4
  },
  "from": "Server",
  "timestamp": 1615820447062666942
},