-
Notifications
You must be signed in to change notification settings - Fork 712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use fewer compilation threads at client when JITServer is low on memory #11323
Conversation
b7f79d6
to
02384da
Compare
@mpirvu ready for review.
|
When I opened the issue I thought we needed more frequent updates. The only danger right now is the server approaching the limit and the client not knowing and launching another comp thread. The part that suspends compilation threads is fine because we cannot react to it until the compilation is over, so the hint from the compilation reply is enough. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few suggestions
@@ -1048,6 +1048,9 @@ class CompilationInfo | |||
void addJITServerSslCert(const std::string &cert) { _sslCerts.push_back(cert); } | |||
const std::string &getJITServerSslRootCerts() const { return _sslRootCerts; } | |||
void setJITServerSslRootCerts(const std::string &cert) { _sslRootCerts = cert; } | |||
|
|||
bool serverHasLowPhysicalMemory() { return _serverHasLowPhysicalMemory; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could mark this one as const
@@ -549,6 +549,10 @@ TR_YesNoMaybe TR::CompilationInfo::shouldActivateNewCompThread() | |||
#if defined(J9VM_OPT_JITSERVER) | |||
else if (getPersistentInfo()->getRemoteCompilationMode() == JITServer::CLIENT && JITServerHelpers::isServerAvailable()) | |||
{ | |||
// If the available memory on the server is low, do not activate more threads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move this piece of code up where the 'NO' decisions are taken. As it is now, we may take the path that leads to TR_yes above.
compInfo->getSuspendThreadDueToLowPhysicalMemory() ? "LowPhysicalMem" : ""); | ||
compInfo->getSuspendThreadDueToLowPhysicalMemory() ? "LowPhysicalMem" : "", | ||
#if defined(J9VM_OPT_JITSERVER) | ||
compInfo->serverHasLowPhysicalMemory() ? "ServerLowPhysicalMem" : "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TR_VerboseLog::writeLineLocked needs another %s for the new string. It gets tricky though due to the ifdef. I am ok with being a little imprecise and say just "LowPhysicalMem", but the decision should come from either the local JVM and or the remote server:
bool lowPhysicalMem = compInfo->getSuspendThreadDueToLowPhysicalMemory();
#if defined(J9VM_OPT_JITSERVER)
lowPhysicalMem = lowPhysicalMem || compInfo->serverHasLowPhysicalMemory();
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, or we could print an empty string if JITServer is not enabled:
compInfo->getRampDownMCT() ? "RampDownMCT" : "",
compInfo->getSuspendThreadDueToLowPhysicalMemory() ? "LowPhysicalMem" : "",
#if defined(J9VM_OPT_JITSERVER)
compInfo->serverHasLowPhysicalMemory() ? "ServerLowPhysicalMem" :
#endif
""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine either way
When the JITServer starts running low on physical memory, it is detrimental for clients to have all compilation threads active and sending requests because it makes JITServer use too many compilation threads which consume a lot of memory. At the end of each compilation, server tells the client if it's low on physical memory. As long as that's the case, client will not activate a new compilation thread and will keep suspending the existing ones until only one thread is active. Signed-off-by: Dmitry Ten <Dmitry.Ten@ibm.com>
02384da
to
1013f40
Compare
@mpirvu updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
jenkins test sanity plinuxjit,xlinuxjit,zlinuxjit jdk11 |
Tests have passed, hence merging. |
When the server is running low on memory, clients need to reduce the number of active compilation threads so that the remaining memory doesn't get exhausted as quickly. Previous implementation in eclipse-openj9#11323 would suspend threads when the server was low on memory, and resume them once more memory becomes available. However, it suffered from constant oscillations in thread activations: i.e. clients would be notified of low server memory, suspend threads, more memory on the server would become available, meaning all the clients would resume their threads, causing the server to run out of memory again, and the cycle would repeat. This commit improves the previous implementation by using 2 new enums to represent client's thread activation policy and server's memory state. `JITServer::CompThreadActivationPolicy` and `JITServer::ServerMemoryState`. `JITServer::ServerMemoryState` has 3 states indicating how much free memory is left on the server. Each client receives an update at the end of each compilation. - `NORMAL`: there is enough memory available - `LOW`: given the current number of clients, the server might run out of memory soon - `VERY_LOW`: the server is about to exhaust its memory and completely stop all compilations. `JITServer::CompThreadActivationPolicy` has 4 states indicating the manner in which client's compilation threads should be suspended/resumed. - `AGGRESSIVE`: the default policy, activates new compilation threads more aggressively than non-JITServer JVM. - `MAINTAIN`: (VERY_LOW < server_memory <= LOW) - server is running out of memory, active threads are allowed to continue but no new threads can start. If server's memory situation improves, thread activation can be resumed. - `SUSPEND`: (server_memory <= VERY_LOW) - server is about to exhaust memory, suspend all but 1 compilation thread. If server's memory improves back to NORMAL, thread activation can be resumed. - `SUBDUE`: (server_memory > LOW) - this policy can be entered from `MAINTAIN` or `SUSPEND` policies. Client is allowed to resume threads but activation thresholds are much higher than in `AGGRESSIVE` policy. Signed-off-by: Dmitry Ten <Dmitry.Ten@ibm.com>
When the JITServer starts running low on physical memory, it is
detrimental for clients to have all compilation threads active and
sending requests because it makes JITServer use too many compilation
threads which consume a lot of memory.
At the end of each compilation, server tells the client if it's low
on physical memory. As long as that's the case, client will not
activate a new compilation thread and will keep suspending the existing
ones until only one thread is active.
Resolves: #11175