search

JetBrains / lincheck

Framework for testing concurrent data structures
Mozilla Public License 2.0
545 stars 31 forks source link

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

Closed eupp closed 4 months ago

eupp commented 4 months ago

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

ndkoval commented 4 months ago

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

eupp commented 4 months ago

The fails are due to #286

eupp commented 4 months ago

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:

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

ndkoval commented 4 months ago

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

eupp commented 4 months ago

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 commented 4 months ago

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