-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
Ability to Dynamically Set CPU Affinities for Benchmarking #1917
Conversation
Using affinity just for one test for now to get started. If these changes look good, I can do the same for other tests in future PR and do proper perf evaluation with a bigger set of tests to show that we do get more reliable results (lower confidence interval) with affinity. Baseline: no affinity Sanity Runsppc64_aix x86-64_mac ppc64le_linux After adding a warning for missing affinity tool s390x_linux s390x_zos aarch64_linux x86-64_linux Disabled Windows x86-64_windows |
x86-64_windows IssueI have to disable Windows temporarily since the test fails without giving any clues. I feel that we are not being able to run the bash script on Windows, which does seem to have cygwin. The output doesn't tell us much as shown below. Here's a build with the changes in this PR except that Windows isn't disabled here.
I tried to look for examples in this repo to see whether any other test is running a shell script on Windows. I notice a couple of external tests (one example below) using bash script and they don't seem to have I don't see any external test pipelines for Windows on Adopt or internal server. I can't launch a grinder either since neither internal or external Jenkins seems to have a Windows machine with docker. x86-64_windows |
Forgot to mention: Currently, we're running Renaissance sanity tests daily while extended tests weekly. If we look at the Tabular View, we can see extremely high confidence interval (i.e. fluctuation in numbers) since they aren't using CPU affinity, while we see quite stable numbers for Quarkus tests, which are using affinity as shown in the screenshot below. |
Documentation:Overview of affinity script: https://github.com/piyush286/openjdk-tests/blob/3db1e3463952cc731fee503673350636d3374f5a/perf/affinity.sh#L18-L93 @smlambert Requesting review from you! I've just used affinity for one test for now because I wanted to check with you to see whether that's the right approach and you're okay with it. Whatever you suggest, we can do that for all the tests. Also, any suggestions regarding the windows issue? Also, I think it would be good to get those affinity tool dependencies added to perf machines if we want to improve our perf testing. Currently, the affinity script can gracefully avoid using affinity commands if the machine doesn't have the dependencies as shown here. Thanks! |
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.
Thanks @piyush286 - wondering if we can temporarily create a second target in the playlist called renaissance-scala-kmeans-windows where we continue to run as before (no run_with_affinity.sh), it serves as a functional test... until the windows issue can be resolved. New target can have a <!-- comment -->
that points to the windows issue raised.
• Added an affinity script that can dynamically set CPU affinities according to the platform and machine HW ○ This script is well-documented. It lists basic usage info, sample commands, common and platform specific dependencies, ○ If some machine doesn't have the affinity tool, this script gracefully handles it. We just output a warning and then continue without using affinity. • Added another script that acts like a wrapper between playlist.xml and affinity script and redirects output of affinity script to /dev/null. ○ It uses 2 physical CPUs with SMT as default, which might be the most common perf testing use case. Hence, we can avoid passing those in the playlist.xml file for every test. In case, we do want to run some test with different HW config, then we can specify them only for that test. Also, if you want to run all tests with different affinity than the default, you can easily change this file for one-off testing. • Used affinity command for just one benchmark to begin with • Added a check to avoid downloading the Renaissance benchmark binary if it already exists Issues: adoptium#1587 adoptium/TKG#34 Signed-off-by: Piyush Gupta <piyush286@gmail.com>
Thanks @smlambert for the review. Updated the changes as suggested. New Testing:x86-64_windows https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/3680/console x86-64_linux |
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.
Thanks @piyush286 - I'll merge once the Grinders finish running (have a great vacation)!
the relaunch on windows looks good https://ci.adoptopenjdk.net/view/Test_grinder/job/Grinder/3681/ so will merge |
Issues:
#1587
adoptium/TKG#34
Signed-off-by: Piyush Gupta piyush286@gmail.com