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

Lincheck benchmarks #250

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

Lincheck benchmarks #250

wants to merge 18 commits into from

Conversation

eupp
Copy link
Collaborator

@eupp eupp commented Nov 20, 2023

This PR adds benchmarking infrastructure to the Lincheck. It includes:

  • API to track progress of lincheck test run: LincheckRunTracker interface.
  • Statistics gathering infrastructure based on LincheckRunTracker API.
  • New gradle tasks to run benchmarks (tests and benchmarks are put into separate source sets).
  • Several benchmarks for standard concurrent data structures and synchronization primitives.
  • Script to draw various plots based on collected benchmark statistics (using Lets-Plot Kotlin library).

@eupp eupp requested a review from ndkoval November 20, 2023 18:57
@ndkoval ndkoval linked an issue Jan 18, 2024 that may be closed by this pull request
@eupp eupp force-pushed the lincheck-benchmarks branch from f6d693d to 23ea160 Compare June 10, 2024 10:37
@eupp eupp force-pushed the lincheck-benchmarks branch from c0e8ce9 to 2b8d4fa Compare June 17, 2024 11:13
@eupp
Copy link
Collaborator Author

eupp commented Jun 26, 2024

Hi @ndkoval !
I've rebased this PR on develop and fixed all previous review comments.
Please have a look.

@eupp eupp requested a review from ndkoval June 26, 2024 14:09
@eupp eupp force-pushed the lincheck-benchmarks branch from d4f3ada to 926ddb2 Compare August 5, 2024 21:44
build.gradle.kts Outdated Show resolved Hide resolved
@ndkoval ndkoval self-requested a review September 24, 2024 09:29
@@ -39,6 +45,51 @@ kotlin {
val test by compilations.getting {
kotlinOptions.jvmTarget = "11"
}

val benchmark by compilations.creating {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please take a look how it is done in the Kotlin coroutines repo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you please point out to some specific part or example of gradle scripts you mean, or describe what is the problem with the current code.

The kotlin coroutines build setup is quite large and complicated, what exactly should I look for?

It looks like they use subprojects approach, and benchmarks, in particular, is one of the subprojects.

Is that what you mean? If yes, what are the benefits of using subprojects for benchmarks compared to having them as another source set?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Making it a subproject makes the build script simpler and modularized, with all the dependencies and hacks necessary for benchmarks being configured for benchmarks only and in a separate file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, it is much shorter 🙃

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, will try it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have tried the sub-projects approach and it haven't worked out.

The problem is that if we extract the benchmarks into a separate sub-project, then we have to add a dependency from benchmarks sub-module to a the lincheck itself.
But, because the lincheck itself is not a sub-project, but a top-level project, we cannot do (i.e., adding dependency { implementation(rootProject) } does not work).

It seems that currently Lincheck project structure does not adhere to the standard recommended project structure, where the top-level build.gradle.kts simply imports sub-projects and defines common configuration, and individual sub-projects' build.gradle.kts files define build scripts for sub-components, like lib, app, benchmarks, etc: https://docs.gradle.org/current/userguide/multi_project_builds.html

The kotlinx-coroutines project also follows this convention, and thus we cannot simply re-use it in Lincheck.
It defines a kotlinx-coroutines-core sub-project with core coroutine library primitives.
The benchmarks sub-project then
declares a dependency on the core subproject.

And it looks like other kotlin libraries follow the same approach, where the top-level build.gradle.kts is only an "umbrella" script and everything else is split into sub-projects. Most libraries also define "core" sub-project, which is either named core or $library-name:

For us it means that we would have to move the current top-level build.gradle.kts file into a new "core" sub-project, which we could name either lincheck, lincheck-core, or core.
But this is a major change, which I think we should implement in a separate PR, not as a part of benchmarks PR.
Also this change is likely to produce a lot of merge-conflicts with existing active feature branches, so we should plan it thoughtfully and coordinate with other folks who currently work on various features in Lincheck.

I have very limited knowledge of gradle, so perhaps there is some way to overcome the problems described above. But I do not know how to do without major refactoring of project structure.

src/jvm/main/org/jetbrains/kotlinx/lincheck/LinChecker.kt Outdated Show resolved Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/LinChecker.kt Outdated Show resolved Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/LinChecker.kt Outdated Show resolved Hide resolved
eupp added 18 commits December 7, 2024 01:29
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
* remove unused `chainTrackers`
* rename `ChainedRunTracker` into `ChainRunTracker`
* improve documentation in `LincheckRunTracker`

Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
@eupp eupp force-pushed the lincheck-benchmarks branch from 1e66117 to bf35150 Compare December 7, 2024 01:15
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.

Create real-world benchmarks
2 participants