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

Add a flag to make get[Process|System]CpuLoad() match RI behaviour #18451

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

thallium
Copy link
Contributor

@thallium thallium commented Nov 15, 2023

Currently, getProcessCpuLoad() and getSystemCpuLoad() return -1 if only one data point has been recorded. A compatibility flag -XX:[+/-]CpuLoadCompatibility is added to match the behaviour of the RI, which is to return 0.

Fixes: #13389
Related: eclipse-omr/omr#7189

@thallium
Copy link
Contributor Author

@TobiAjila FYI

@tajila tajila self-requested a review November 15, 2023 02:29
@tajila
Copy link
Contributor

tajila commented Nov 15, 2023

@ChengJin01 Please review these changes

@tajila tajila requested a review from ChengJin01 November 15, 2023 15:34
Copy link

@ChengJin01 ChengJin01 left a comment

Choose a reason for hiding this comment

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

Approve the PR from code perspective assuming the description & comments are appended with ..

thallium added a commit to thallium/omr that referenced this pull request Nov 22, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Nov 22, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Nov 23, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Nov 24, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Nov 27, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Nov 27, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Nov 27, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Nov 27, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Dec 5, 2023
This is to help getSystemCpuLoad() determine if it's the first call.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Dec 6, 2023
…e data point is available

This is to help getSystemCpuLoad() determine if it's the first call.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Dec 6, 2023
This is to determine if a call to omrsysinfo_get_CPU_load has onlyone data point recorded.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Dec 6, 2023
This is to determine if a call to omrsysinfo_get_CPU_load has onlyone
data point recorded.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
thallium added a commit to thallium/omr that referenced this pull request Dec 6, 2023
This is to determine if a call to omrsysinfo_get_CPU_load has only one
data point recorded.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
@tajila
Copy link
Contributor

tajila commented Dec 11, 2023

@thallium Please rebase this PR

@thallium thallium force-pushed the getProcessCpuLoad branch 2 times, most recently from 466c2ff to d2f5475 Compare December 12, 2023 16:22
@thallium
Copy link
Contributor Author

thallium commented Dec 12, 2023

@tajila Rebased

@tajila
Copy link
Contributor

tajila commented Dec 12, 2023

jenkins test sanity alinux64 jdk17

@tajila
Copy link
Contributor

tajila commented Dec 12, 2023

jenkins test sanity win jdk8

@tajila
Copy link
Contributor

tajila commented Dec 13, 2023

@thallium there are some test failures

@thallium
Copy link
Contributor Author

@tajila There's a related test testing the return value of first call. Should we change it to zero or remove it?

@tajila
Copy link
Contributor

tajila commented Dec 13, 2023

See #13389 (comment)

Currently, getProcessCpuLoad() and getSystemCpuLoad() return -1 if only
one data point has been recorded. A compatibility flag
-XX:[+/-]CpuLoadCompatibility is added to match the behaviour of the RI,
which is to return 0.

Fixes: eclipse-openj9#13389
Related: eclipse-omr/omr#7189

Signed-off-by: Gengchen Tuo <[email protected]>
@thallium thallium changed the title Make get[Process|System]CpuLoad() return 0 on the first call Add a flag to make get[Process|System]CpuLoad() return 0 on the first call Dec 13, 2023
@thallium
Copy link
Contributor Author

Fixed the logic and commit message.

@tajila
Copy link
Contributor

tajila commented Dec 13, 2023

jenkins test sanity alinux64 jdk17

@tajila tajila changed the title Add a flag to make get[Process|System]CpuLoad() return 0 on the first call Add a flag to make get[Process|System]CpuLoad() match RI behaviour Dec 15, 2023
@tajila tajila merged commit db584a0 into eclipse-openj9:master Dec 15, 2023
@tajila
Copy link
Contributor

tajila commented Dec 15, 2023

@thallium Can you please open a docs issue

@pshipton
Copy link
Member

Reverted via #18635

thallium added a commit to thallium/omr that referenced this pull request Dec 17, 2023
This is to determine if a call to omrsysinfo_get_CPU_load has only one
data point recorded.

Also fixed a mistake in the description saying that the first two
invocations return OMRPORT_ERROR_OPFAILED and fix the time unit in the
description to "ms".

Related: eclipse-openj9/openj9#18451

Signed-off-by: Gengchen Tuo <[email protected]>
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.

getProcessCpuLoad() always return -1 in first call on jdk11
4 participants