Skip to content

Conversation

@ASalavei
Copy link

Note: The change has been already reviewer in another MR, but failed to merge because of the merging issues.

Remove inflightSemaphore because does not prevent from awaiting the main thread on the nextDrawable().
Replace InflightCommandBuffers and replace it with the Dispatch Group.

Fixes https://youtrack.jetbrains.com/issue/CMP-9494/Optimize-MetalRedrawer-synchronization

Release Notes

N/A

@ASalavei
Copy link
Author

@junie-agent

Your Task

  1. Download the Pull Request diff using:
    gh pr diff ${{ github.event.pull_request.head.ref }}

  2. Review only:

  • the code changed in the diff
  • code that is directly or closely connected to the changed code
    Do not explore other parts of the repository.
  1. Perform the review according to the criteria below.
  2. Write GitHub comment for every issue you found.
  3. Do not write any summary about the pull request. Only the issues you found.

You MUST NOT:

  • run the code
  • build the project
  • execute tests
  • explore the repository beyond the files included in the diff.
  • make any changes in the code review
  • commit anything into the code review branch
  • write code review summary

Review Scope

1. Comments

Use professional and polite language.

  • Add a comment if you are absolutely sure about an issue you have found in the code; otherwise, skip it.
  • Do not write comments if you are unsure about an issue in the code.
  • Do not write a summary of the code review. Only write comments on specific issues if they are identified.
  • Only write about issues that you have found. Don't write anything else.
  • If issues found, write comments using the flolwing output format:

⚠️ 2 issues found

  1. [full path to the issue file] : [line number]
    [short comment for the issue 1]

  2. [full path to the issue file] : [line number]
    [short comment for the issue 2]

  • If no issues found, write only this line and nothing else:

✅ No issues found

2. Check criteria

  • Verify that the code changes corresponding the PR title and description.
  • Verify any grammar mistakes in the diff. Don't highlight grammar mistakes outside the pull request's diff.
  • Use deep knowledge about algorithms and data structures to find potential issues in the code.
  • Deeply analyze logic of algorithms that are changed in the PR and find inconsistencies between the implementation and expected behaviour.
  • Check for common typos in the code logic, like == vs !=.
  • Any other issues that crucial for the code review.

@github-actions
Copy link

github-actions bot commented Jan 19, 2026

Junie successfully finished!

Result: Code Review for MetalRedrawer Synchronization Optimization
The pull request "Optimize MetalRedrawer synchronization" was reviewed. The review identified a potential race condition in the synchronization logic when the application transitions to the background and an outdated variable name and description in a comment.

⚠️ 2 issues found

  1. compose/ui/ui/src/iosMain/kotlin/androidx/compose/ui/window/MetalRedrawer.ios.kt : 235
    When the application moves to the background, dispatch_group_wait is used to wait for in-flight command buffers. However, dispatch_group_enter is called inside encodeAndPresentBlock, which may be dispatched asynchronously to the renderingDispatchQueue. If the block has been dispatched but not yet started execution, the inflightCommandBuffersGroup will be empty, and dispatch_group_wait will return immediately. This creates a race condition where GPU work can be committed after the application has transitioned to the background, potentially violating Metal's background execution rules.

  2. compose/ui/ui/src/iosMain/kotlin/androidx/compose/ui/window/MetalRedrawer.ios.kt : 232
    The comment refers to inflightCommandBuffers, which is the name of the variable that was removed in this PR. It should be updated to inflightCommandBuffersGroup. Additionally, the comment says "synchronously schedule", but the code actually waits for the command buffers to complete. A more accurate description would be "synchronously wait for all in-flight command buffers to complete".

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