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

Provide a way to manually create SampleAnalysisEnvironment #3573

Closed
IgnatBeresnev opened this issue Apr 22, 2024 · 1 comment · Fixed by #3589
Closed

Provide a way to manually create SampleAnalysisEnvironment #3573

IgnatBeresnev opened this issue Apr 22, 2024 · 1 comment · Fixed by #3589
Assignees
Labels
feedback: Google An issue/PR submitted by colleagues at Google, most likely related to the Android API reference docs language: Kotlin Issue/PR related to the Kotlin language feature/analysis/docs

Comments

@IgnatBeresnev
Copy link
Member

Context

The API introduced for analyzing samples in #3195 does not fit Dackka (Google's plugin) well: Dokka works through processing batches of documentables in a pipeline, but Dackka has more of a service-like architecture, where it can invoke sample resolution from later stages.

Problem

Invoking SampleAnalysisEnvironmentCreator#use for every resolution request in Dackka leads to stalling and other problems. Refactoring the architecture to batch sample resolution is also tricky.

Solution

The following API was discussed with Google, and tested on Dackka, which seems to work and solve the problem stated above.

  1. Make SampleAnalysisEnvironment closeable (java.io.Closeable).
  2. Add public fun create(): SampleAnalysisEnvironment to SampleAnalysisEnvironmentCreator

The commit that has these changes: 2b6eb92 (sample-analysis-manual branch)

In order to merge these changes in, the following needs to be done:

  1. Add a KDoc to SampleAnalysisEnvironmentCreator#create, stating that:
    • it's delicate API and should be avoided in favor of #use, because it can lead to memory leaks, concurrency issues or other unexpected problems, whereas use has a controlled environment.
    • the SampleAnalysisEnvironment that is created should be closed by the user manually, otherwise it may lead to leaks
  2. Ideally, an opt-in annotation like @DelicateDokkaApi should be added, similar to DelicateCoroutinesApi.
  3. Add unit tests (there is one test already, but maybe more are needed)

Other

The branch sample-analysis-manual may be revived, although it's not a requirement - a new branch can be created.

@IgnatBeresnev IgnatBeresnev added language: Kotlin Issue/PR related to the Kotlin language feature/analysis/docs feedback: Google An issue/PR submitted by colleagues at Google, most likely related to the Android API reference docs labels Apr 22, 2024
@whyoleg
Copy link
Contributor

whyoleg commented Apr 24, 2024

Idea: may be we can also track created SampleAnalysisEnvironment via create and validate (and close) them in PostAction, like we do with closing of analysis session?
F.e we could log a warning in PostAction, that there is some SampleAnalysisEnvironment cerated and not closed. This should be rather small change which could help with both mis-use of API and reducing possible memory leaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback: Google An issue/PR submitted by colleagues at Google, most likely related to the Android API reference docs language: Kotlin Issue/PR related to the Kotlin language feature/analysis/docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants