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
8304825: MacOS metal pipeline - window isn't painted if created during display sleep #13230
Conversation
…g display sleep Use one display link thread per MTLContext. Adjust refresh rate with current display.
👋 Welcome back avu! A progress list of the required criteria for merging this PR into |
I tested the test program of this bug and SwingSet2 - with and without external monitor attached.
We need to address the performance degradation that this patch causes or look for a simpler solution. |
Thanks for the testing. I've run only RenderPerf and only on M1 but it didn't show any regressions (there were some improvements actually) I've removed the dedicated blitCommandQueue because it caused the unpredicted order of rendered frames to appear on the screen. In general, it's recommended to use only one command queue per application. If we still want to use a separate queue, we need to provide some synchronization here https://developer.apple.com/documentation/metal/resource_synchronization/synchronizing_events_within_a_single_device?language=objc . |
@aghaisas could you return back the blit command queue in my patch and see if it resolves the performance issue with SwingMark? I'm not completely sure that it's the main reason for the performance degradation. Another possible reason is the increased amount of start/stop displaylink calls because of changes in the logic that manages them. |
I modified the patch to add back
This testing indicates that there is an additional performance hit due to the change in logic apart from |
I suggest we fix this bug on by either of the two options below-
I have implemented and tested Option 1. I have not tried Option 2 yet. The code refactoring that you have done can be done as a separate JBS issue if needed in future. What do you think? |
OK, let's try Option 1. It still may be useful to recreate CVDisplaylink on the fly, for example if macOS switch GPU from internal to discrete one. I suppose my changes could be adjusted with this approach and pushed as a separate PR. |
@aghaisas what particular test showed the regression, I've tested x64 and aarch64 macs with different tests from J2DBench and haven't found regressions yet. Also, could you provide your J2DBench options file? |
Those are the test batches run on internal servers. The results that prompted my review comments- **J2DBench test related to opaque image rendering **Compared to the current mainline metal rendering pipeline performance -
Swingmark test shows -
|
Use one display link thread per MTLContext. Adjust the refresh rate with the current display.
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13230/head:pull/13230
$ git checkout pull/13230
Update a local copy of the PR:
$ git checkout pull/13230
$ git pull https://git.openjdk.org/jdk.git pull/13230/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 13230
View PR using the GUI difftool:
$ git pr show -t 13230
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13230.diff
Webrev
Link to Webrev Comment