Add error check for checkpointGCThreads #18617
Conversation
Add error check in gcParseXXArguments() for -XX:CheckpointGCThread= to make sure it cannot be larger than the existing gcthreads value. Signed off by: [email protected]
|
The docs mention, https://eclipse.dev/openj9/docs/xxcheckpointgcthread/ Do you know where this is done? |
|
Also, this behaviour needs a warning message |
| @@ -1439,6 +1439,10 @@ gcParseXXArguments(J9JavaVM *vm) | |||
| j9nls_printf(PORTLIB, J9NLS_ERROR, J9NLS_GC_OPTIONS_VALUE_MUST_BE_ABOVE, "-XX:CheckpointGCThreads=", (UDATA)0); | |||
| goto _error; | |||
| } | |||
| if (extensions->checkpointGCthreadCount > extensions->gcThreadCount) { | |||
| j9nls_printf(PORTLIB, J9NLS_ERROR, J9NLS_GC_OPTIONS_MUST_BE_NO_GREATER_THAN, "-XX:CheckpointGCThreads=", VMOPT_XGCTHREADS); | |||
| goto _error; | |||
There was a problem hiding this comment.
The JVM shouldn't fail, but a warning message should be printed indicating that the option wasn't respected.
|
At the moment this change is incomplete/incorrect since it's trying to address only the case when both CheckpointGCThread and Xgcthreads option are specified. There are other scenarios where one option is specified and the other is not, and for some of those the checks need to be on other places in code, if we want to issue an warning message. For example, if gcthreads is not specified, the count is determined at a later point, after the parsing is done. And if CheckpointGCThread is specified, the current check will actually detect that CheckpointGCThread > gcthreads, but only because gcthreads is still 0. If I understand correctly, how we currently deal with CheckpointGCThread > gcthreads scerario, is that we just silently adjust CheckpointGCThread to be equal to gcthreads. Simply, we do not do any gcthread pool contraction during prepare-for-checkpoint stage, and leave the gcthread pool intact. Perhaps a simpler approach is just to issue a warning/informational message during that prepare-for-checkpoint stage if we contracted (and to what count) the gcthread pool? We can consider it to be a message be a part of VGC or just a general stderr one.... The is different from the attempted approach to detect/warn about this inequality during startup phase of checkpoint run. Perhpas still a valid approach, but more slightly more complex since more spots in the code have to be covered.... |
|
close this as a new one to warn has been created |
Add error check in gcParseXXArguments() for -XX:CheckpointGCThread=
to make sure it cannot be larger than the existing gcthreads value.
Signed off by: [email protected]