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

Fix number of iterations in spin-loops in case if #cpus < #threads #284

Merged
merged 21 commits into from
Mar 7, 2024

Conversation

eupp
Copy link
Collaborator

@eupp eupp commented Feb 29, 2024

Currently, Lincheck performs extremely slow on the machines where number of CPUs is less than a number of threads in the tested scenario.

The reason for that is because there are several places in the Lincheck codebase where spin-loops are used to wait for a certain condition. These loops spin a fixed (large) number of times. But if there are no available CPUs, then these spin-loops just consume all the available computational resources and the test hangs.

Previously, only in one of the places where spin-loops are utilized there was implemented a special check for the case when #cpus < #threads. Because of code duplication, other similar places were omitted by mistake.

To address this issue, I extracted common spin-looping logic into an utility class Spinner (inspired by the similar solutions in the Rust's crossbeam and parking-lot libraries).

This class also has a special handling for the case when #cpus < #threads to avoid redundant spinning.

In future, we might consider to improve the performance of the spinning by utilizing the exponential backoff strategy, similarly as how it is done in the crossbeam library.

Closes #280

@eupp eupp requested a review from ndkoval February 29, 2024 22:43
@eupp eupp requested a review from ndkoval March 4, 2024 13:33
@ndkoval
Copy link
Collaborator

ndkoval commented Mar 4, 2024

@eupp please investigate why builds on Teamcity take longer and fail periodically with this change.

@eupp
Copy link
Collaborator Author

eupp commented Mar 4, 2024

The fails are due to #286

@eupp
Copy link
Collaborator Author

eupp commented Mar 6, 2024

As for the performance degradation, it does not look like this is a stable performance degradation.

For example, I just re-run Java 17 builds on master, develop and fix-spin-loops branches, and here are results:

  • master (last build): 17m 51s
  • develop (last build): 20m 01s
  • fix-spin-loops (last build): 16m 51s

If we take last 6 builds of develop (starting from the last branch-off from master), the build time varies from 13m to 20m.

Signed-off-by: Evgeniy Moiseenko <[email protected]>
@eupp eupp requested a review from ndkoval March 6, 2024 23:15
if (suddenInvocationResult != null) throw ForcibleExecutionFinishError
if (++i % SPINNING_LOOP_ITERATIONS_BEFORE_YIELD == 0) Thread.yield()
if (suddenInvocationResult != null)
throw ForcibleExecutionFinishError
Copy link
Collaborator

Choose a reason for hiding this comment

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

please put throw on the same line

@ndkoval
Copy link
Collaborator

ndkoval commented Mar 7, 2024

@eupp I've approved the PR. Please fix the minor issue and merge it.

Signed-off-by: Evgeniy Moiseenko <[email protected]>
@eupp
Copy link
Collaborator Author

eupp commented Mar 7, 2024

Apparently, I cannot merge this until all CI checks are passed : )
If you have a right to bypass this check, please do.
Otherwise, I am gonna try to fix #286 first.

@eupp
Copy link
Collaborator Author

eupp commented Mar 7, 2024

Oh, nevermind, I have just by-passed this problem.

@eupp eupp merged commit 5c8579c into develop Mar 7, 2024
12 checks passed
@eupp eupp deleted the fix-spin-loops branch March 7, 2024 12:45
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.

2 participants