Skip to content
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

[Misc] Return suggested value for constraint funcs #878

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ziyilin
Copy link

@ziyilin ziyilin commented Oct 28, 2024

Verify JVM parameters, print out suggested values for violations instead of error message.

Enable by -XX:+VerifyFlagConstraints.

@CLAassistant
Copy link

CLAassistant commented Oct 28, 2024

CLA assistant check
All committers have signed the CLA.

@ziyilin ziyilin force-pushed the printConstraintFuncs branch from 7377313 to ab920bd Compare October 29, 2024 08:35
@ziyilin ziyilin changed the title Print constraint funcs [Misc] Return suggested value for constraint funcs Nov 26, 2024
@ziyilin ziyilin force-pushed the printConstraintFuncs branch 2 times, most recently from aa883b3 to bda9bf3 Compare November 26, 2024 08:55
diagnostic(bool, VerifyFlagConstraints, false, \
"Verify flags, calculate suggested legal values for that " \
"violate the constraints, and then exit VM") \
\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我们添加的参数建议放在globals_ext.hpp中

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -36,6 +36,10 @@
static JVMFlag::Error ParallelGCThreadsAndCMSWorkQueueDrainThreshold(uint threads, uintx threshold, bool verbose) {
// CMSWorkQueueDrainThreshold is verified to be less than max_juint
if (UseConcMarkSweepGC && (threads > (uint)(max_jint / (uint)threshold))) {
if (VerifyFlagConstraints) {
//JVMFlag::printError(true, "ParallelGCThreads:"UINT32_FORMAT"\n", )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里为什么不修复并报错,而是直接返回了

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

目前先不支持CMS,我把这里的检查去掉了


if (status == JVMFlag::SUCCESS) {
status = CheckMaxHeapSizeAndSoftRefLRUPolicyMSPerMB(value, SoftRefLRUPolicyMSPerMB, verbose);
if ((SoftRefLRUPolicyMSPerMB > 0) && ((value / M) > (max_uintx / SoftRefLRUPolicyMSPerMB))) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么这里要展开这个方法?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckMaxHeapSizeAndSoftRefLRUPolicyMSPerMB原先被SoftRefLRUPolicyMSPerMBConstraintFuncMaxHeapSizeConstraintFunc公用,原先分别在两个use里展开就不用检查来源了。不过还是应该提高复用度,已经修改了CheckMaxHeapSizeAndSoftRefLRUPolicyMSPerMB函数的声明,增加了来源。

@@ -50,6 +50,7 @@
#ifdef COMPILER2
#include "opto/c2_globals.hpp"
#endif // COMPILER2
#include <cmath>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件是必须要加的吗

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -29,22 +29,39 @@
#include "runtime/globals.hpp"
#include "runtime/safepointMechanism.hpp"
#include "runtime/task.hpp"
#include <cmath>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要引入这个文件

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

if (VerifyFlagConstraints) {
verifyFailed = true;
int logValue = log2_intptr(value);
value = (intx)pow(2, logValue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改成 1 << logValue 即可

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

if (VerifyFlagConstraints) {
verifyFailed = true;
int logValue = log2_intptr(value);
value = (intx)pow(2, logValue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改成 1 << logValue

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -33,9 +33,15 @@
#include "runtime/flags/jvmFlagConstraintsCompiler.hpp"
#include "runtime/globals.hpp"
#include "runtime/globals_extension.hpp"
#include <cmath>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要引入这个文件

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

return JVMFlag::VIOLATES_CONSTRAINT;
}

//InvocationCounter::count_shift = 3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this line

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

"larger than or equal to CodeEntryAlignment (" INTX_FORMAT ") "
"to align entry points\n",
CodeCacheSegmentSize, CodeEntryAlignment);
"CodeCacheSegmentSize (" UINTX_FORMAT ") must be "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不用修改格式

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -241,6 +299,7 @@ JVMFlag::Error CompilerThreadPriorityConstraintFunc(intx value, bool verbose) {
}

JVMFlag::Error CodeEntryAlignmentConstraintFunc(intx value, bool verbose) {
// Don't apply VerifyFlagConstraints here because CodeEntryAlignment is not production parameter.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个方法的改动好像都不需要

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ziyilin ziyilin force-pushed the printConstraintFuncs branch from bda9bf3 to b204782 Compare December 5, 2024 06:21
Summary: Return suggested option value when it violates the constraints.
Exit the VM after all input options are checked.
Testing: JTreg
Reviewers: Kuaiwei, wenjie
Issue: dragonwell-project#894
@ziyilin ziyilin force-pushed the printConstraintFuncs branch from b204782 to 92d3bd5 Compare December 5, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants