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

Leverage Chronicle Tune isolate information when assigning cores #107

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

tgd
Copy link
Contributor

@tgd tgd commented Aug 22, 2023

Integrates Affinity with Chronicle Tune isolate configuration so that Affinity plays nicely with Tune out of the box. With this change, if isolate is installed and cores isolated only cores that are isolated will be eligible for pinning. Please take a look and let me know your thoughts.

README.adoc Outdated
@@ -185,6 +185,10 @@ $ for i in "$(ls cpu-*)";

----

=== Chronicle Tune integration

https://chronicle.software/tune/[Chronicle Tune] is software that automatically configures your OS for optimal performance. Chronicle Tune can be configured to isolate cores for running low-latency workloads. If Chronicle Tune is actively configured on the target system then Java-Thread-Affinity will automatically load the Tune configuration and record which cores have been isolated. In order to ensure the best performance and the least surprises out of the box Affinity will only consider locking workloads to cores isolated via Chronicle Tune. If Chronicle Tune is installed but no cores are isolated then Affinity will consider all cores as potential lock candidates.
Copy link
Contributor

Choose a reason for hiding this comment

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

pls remove the word "automatically" from the first sentence

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest add "also" to " Chronicle Tune can also be configured"....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - that's updated

@@ -400,6 +407,11 @@ public void bind(boolean wholeCore) {

final boolean canReserve(boolean specified) {

if (isolationSettingsPreventUse()) {
LOGGER.debug("Cannot reserve CPU {} because it is not isolated by Chronicle Tune", cpuId);
Copy link
Contributor

@JerryShea JerryShea Aug 24, 2023

Choose a reason for hiding this comment

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

info? warn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - that's updated to cover the two different use cases in which this might get called

Copy link
Contributor

@JerryShea JerryShea left a comment

Choose a reason for hiding this comment

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

nice!

@peter-lawrey peter-lawrey requested review from peter-lawrey and removed request for peter-k-lawrey March 5, 2024 11:46
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.

None yet

5 participants