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

Use Worker API for Benchmark generators #235

Merged
merged 5 commits into from
Jul 10, 2024
Merged

Conversation

adam-enko
Copy link
Member

@adam-enko adam-enko commented Jul 4, 2024

Use Worker API to generate benchmark code

  • Update the Benchmark JS/Wasm/Native source generation tasks to use the Worker API, so that kotlin-compiler-embeddable can be a compileOnly dependency, thus resolving https://youtrack.jetbrains.com/issue/KT-66764
  • Create GenerateJsSourceWorker and GenerateWasmSourceWorker for isolating the JS and Wasm generation code.
  • Updated the Js/Wasm/Native generation tasks to invoke the source generators in a Worker isolated classpath.
  • Created a new Configuration for declaring kotlin-compiler-embeddable, and pass it to the Js/Wasm/Native generation tasks.
  • Create a BenchmarkDependencies utility for defining all Configurations used in the plugin. (This also lays the groundwork for unifying the JMH version Update JMH version #147 (comment))
  • Updated BenchmarksPluginConstants to add DEFAULT_KOTLIN_COMPILER_VERSION, as sensible default for kotlin-compiler-embeddable.
  • Add opt-in annotation RequiresKotlinCompilerEmbeddable to highlight code that requires kotlin-compiler-embeddable
  • Some classes were updated to be abstract, to follow Gradle best practices for creating managed objects.

I have tested this manually by running the Gradle Plugin tests with Kotlin version set to 2.0. However, Benchmarks is not compatible with KGP 2.0 as a KGP function has changed #236. If I comment out this changed function, then Benchmarks works as expected.

@adam-enko adam-enko changed the base branch from master to adam/feat/convert-gradle-to-kts July 4, 2024 07:24
@adam-enko adam-enko force-pushed the adam/feat/use-worker-api branch 7 times, most recently from e5f0f46 to 70c4cab Compare July 4, 2024 10:49
- Update the Benchmark JS/Wasm/Native source generation tasks to use the Worker API, so that `kotlin-compiler-embeddable` can be a compileOnly dependency, thus resolving https://youtrack.jetbrains.com/issue/KT-66764
- Create `GenerateJsSourceWorker` and `GenerateWasmSourceWorker` for isolating the JS and Wasm generation code.
- Updated the Js/Wasm/Native generation tasks to invoke the source generators in a Worker isolated classpath.
- Created a new Configuration for declaring `kotlin-compiler-embeddable`, and pass it to the Js/Wasm/Native generation tasks.
- Create a BenchmarkDependencies utility for defining all Configurations used in the plugin. (This also lays the groundwork for unifying the JMH version #147 (comment))
- Updated BenchmarksPluginConstants to add `DEFAULT_KOTLIN_COMPILER_VERSION`, as sensible default for kotlin-compiler-embeddable.
- Add opt-in annotation RequiresKotlinCompilerEmbeddable to highlight code that requires `kotlin-compiler-embeddable`
- Some classes were updated to be `abstract`, to follow Gradle best practices for creating managed objects.

This commit also contains formatting changes, as the code style is not consistent.
@adam-enko adam-enko changed the base branch from adam/feat/convert-gradle-to-kts to master July 4, 2024 13:20
@adam-enko adam-enko marked this pull request as ready for review July 4, 2024 15:22
@fzhinkin fzhinkin self-requested a review July 8, 2024 12:45
@fzhinkin
Copy link
Contributor

fzhinkin commented Jul 9, 2024

Everything looks good to me; only need to revamp kotlinx-benchmark's CI before merging.

Copy link

@dkrasnoff dkrasnoff left a comment

Choose a reason for hiding this comment

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

Good job!

@fzhinkin
Copy link
Contributor

@adam-enko please update the branch with the latest changes from master and once the CI is green the PR will be merged

@adam-enko
Copy link
Member Author

@adam-enko please update the branch with the latest changes from master and once the CI is green the PR will be merged

Done!

Will the PR be blocked because your last GitHub review is 'requested changes'?

@adam-enko adam-enko merged commit 0764551 into master Jul 10, 2024
@adam-enko adam-enko deleted the adam/feat/use-worker-api branch July 10, 2024 13:59
kx412764776 pushed a commit to kx412764776/kotlinx-benchmark that referenced this pull request Jul 11, 2024
* Use Worker API to generate benchmark code

- Update the Benchmark JS/Wasm/Native source generation tasks to use the Worker API, so that `kotlin-compiler-embeddable` can be a compileOnly dependency, thus resolving https://youtrack.jetbrains.com/issue/KT-66764
- Create `GenerateJsSourceWorker` and `GenerateWasmSourceWorker` for isolating the JS and Wasm generation code.
- Updated the Js/Wasm/Native generation tasks to invoke the source generators in a Worker isolated classpath.
- Created a new Configuration for declaring `kotlin-compiler-embeddable`, and pass it to the Js/Wasm/Native generation tasks.
- Create a BenchmarkDependencies utility for defining all Configurations used in the plugin. (This also lays the groundwork for unifying the JMH version Kotlin#147 (comment))
- Updated BenchmarksPluginConstants to add `DEFAULT_KOTLIN_COMPILER_VERSION`, as sensible default for kotlin-compiler-embeddable.
- Add opt-in annotation RequiresKotlinCompilerEmbeddable to highlight code that requires `kotlin-compiler-embeddable`
- Some classes were updated to be `abstract`, to follow Gradle best practices for creating managed objects.

This commit also contains formatting changes, as the code style is not consistent.

* Replace `Kotlinx Benchmark` with `kotlinx-benchmark` in docs
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.

3 participants