Add timestamps, queue size and cleared caches information in diagnostic Thread #12746
Conversation
|
@mpirvu Can you please review my PR? Thanks. |
| @@ -803,6 +803,8 @@ class CompilationInfo | |||
| void incNumCompThreadsActive() { _numCompThreadsActive++; } | |||
| void decNumCompThreadsActive() { _numCompThreadsActive--; } | |||
| void setNumCompThreadsActive(int32_t n) { _numCompThreadsActive = n; } | |||
| int32_t getNumClearedCaches() const { return _numClearedCaches; } | |||
There was a problem hiding this comment.
This information is specific to JITServer, so it should stay in a block of code protected by ifdef.
| @@ -1160,6 +1162,7 @@ class CompilationInfo | |||
| intptr_t _numSyncCompilations; | |||
| intptr_t _numAsyncCompilations; | |||
| int32_t _numCompThreadsActive; | |||
| int32_t _numClearedCaches; | |||
There was a problem hiding this comment.
A short comment would be nice here: "number of instances JITServer was forced to clear its internal per-client caches"
| @@ -262,6 +262,8 @@ TR::CompilationInfoPerThreadRemote::waitForMyTurn(ClientSessionData *clientSessi | |||
| TR_ASSERT(entry._compInfoPT == this, "Must have stored compInfoPT into the entry to be compiled"); | |||
| uint32_t seqNo = getSeqNo(); | |||
| uint32_t criticalSeqNo = getExpectedSeqNo(); // This is the seqNo that I must wait for | |||
| TR::CompilationInfo *compInfo = getCompilationInfo(); | |||
There was a problem hiding this comment.
I am thinking that, if you make the new counter a static instance of TR::CompilationInfoPerThreadRemote then you don't have to get hold of getCompilationInfo
| @@ -334,6 +336,7 @@ TR::CompilationInfoPerThreadRemote::waitForMyTurn(ClientSessionData *clientSessi | |||
| !getWaitToBeNotified()) // Avoid a cohort of threads clearing the caches | |||
| { | |||
| clientSession->clearCaches(); | |||
| compInfo->setNumClearedCaches(++numClearedCaches); | |||
There was a problem hiding this comment.
This won't work because there could be a large time span between the moment you read the counter at the beginning of this function and the current moment where you increment it. These two operations need to stay close to one another. Better still, you can create a function that increments it: incNumClearedCaches()
| @@ -112,10 +113,17 @@ static int32_t J9THREAD_PROC statisticsThreadProc(void * entryarg) | |||
| avgCpuUsage = cpuUtil->getAvgCpuUsage(); | |||
| vmCpuUsage = cpuUtil->getVmCpuUsage(); | |||
| } | |||
| OMRPORT_ACCESS_FROM_J9PORT(PORTLIB); | |||
There was a problem hiding this comment.
I don't know what the overhead of this is, but I believe it can be moved outside the loop, near PORT_ACCESS_FROM_JAVAVM(vm);. BTW do we need both PORT_ACCESS and OMRPORT_ACCESS?
There was a problem hiding this comment.
Yes, we need OMRPORT_ACCESS for omrstr_ftime_ex function
EmanElsaban
force-pushed
the
improveHeartBeatThread
branch
from
d371676
to
27f8728
Compare
…ic thread Added timestamps in the diagnostic thread to know when the information printed occurred. In addition, I added the compilation queue size and the number of times the server clears the clientSessionData/persistent caches. Signed-off-by: Eman Elsabban <eman.elsaban1@gmail.com>
EmanElsaban
force-pushed
the
improveHeartBeatThread
branch
from
27f8728
to
557716c
Compare
|
jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit jdk11 |
Added timestamps in the diagnostic thread to know when the information printed occurred.
In addition, I added the compilation queue size and the number of times the server clears the
clientSessionData/persistent caches.
issue: #12658
Signed-off-by: Eman Elsabban eman.elsaban1@gmail.com