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.

Sign up for GitHub

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

Jump to bottom

Add option to skip blit of virtual swapchain #1156

Merged
merged 3 commits into from
Mar 29, 2024
Merged

Add option to skip blit of virtual swapchain #1156

merged 3 commits into from
Mar 29, 2024

Conversation

marius-pelegrin-arm
Copy link
Contributor

Add an option during replay to skip blitting virtual swapchain to real swapchain to remove the overhead of the blit. This overhead is noticeable for microbenchmarks and impacts hardware counter analysis.
The option is "--vssb" or "--virtual-swapchain-skip-blit".

@ci-tester-lunarg
Copy link

Author arm-marius-pelegrin not on autobuild list. Waiting for curator authorization before starting CI build.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 20198.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 2872 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 20249.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 2875 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 2875 passed.

@ci-tester-lunarg
Copy link

Author marius-pelegrin-arm not on autobuild list. Waiting for curator authorization before starting CI build.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 1751.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 2933 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 2933 passed.

@bradgrantham-lunarg
Copy link
Contributor

Do I understand this change correctly to only call Present if performance_mode_ is true? So it's equivalent to wrapping all the Virtual Swapchain logic with if(!performance_mode_)? I think calling Present also causes all the implicit barriers and transitions that you'd expect for the chosen swapchain image, but bypassing all the Virtual Swapchain logic means it will wait on the wrong resources, wouldn't it, if the swapchain image index returned during replay is not the same as the index returned during capture? I'm not sure this is what I expect from "skip blit". Can you give me more of a summary of what you expect from any end-of-frame synchronization that would normally happen in Present and have you tried this with present modes MAILBOX and IMMEDIATE?

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 5765.

@marius-pelegrin-arm
Copy link
Contributor Author

Hi ! To fix this, here's a little change, to skip ONLY the blit. This way the synchronizations and image layouts still behave the same.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 2975 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 2975 failed.

@bradgrantham-lunarg bradgrantham-lunarg added the P2 A high-priority code maintenance issue or a functional problem that is recoverable or not a crash. label Oct 30, 2023
@marius-pelegrin-arm marius-pelegrin-arm force-pushed the virtual-swapchain-skip-blit branch from efe6a6b to 6c56ff1 Compare January 25, 2024 15:51
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 122319.

@marius-pelegrin-arm marius-pelegrin-arm force-pushed the virtual-swapchain-skip-blit branch from 6c56ff1 to 992f714 Compare January 25, 2024 16:08
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 122334.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3694 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3694 failed.

@bradgrantham-lunarg
Copy link
Contributor

Probably this failure in 3694 is an internal CI issue related to the other failures today.

However, @marius-pelegrin-arm @per-mathisen-arm have you tried out --swapchain offscreen for replay, which doesn't QueuePresent or do you require creating the onscreen surfaces?

@per-mathisen-arm
Copy link
Contributor

We would like to replay it with a performance workload that is as close as possible to the real app, and I think offscreen is quite a bit off that ideal. As far as I understood we'd be missing the final blit to the swapchain entirely with offscreen. On the other hand, using virtual swapchain with this patch approximates very closely to the workload of running without virtual swapchain.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 141005.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3773 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3773 failed.

@MarkY-LunarG MarkY-LunarG self-requested a review March 5, 2024 23:41
@MarkY-LunarG MarkY-LunarG force-pushed the virtual-swapchain-skip-blit branch from 992f714 to 749929a Compare March 6, 2024 16:17
@lunarpapillo
Copy link
Contributor

CI failure seems to be due to a busy machine. Re-running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3819 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3819 passed.

@charles-lunarg charles-lunarg force-pushed the virtual-swapchain-skip-blit branch from 749929a to 1593bf2 Compare March 22, 2024 04:28
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 154811.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3899 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 154823.

charles-lunarg
charles-lunarg approved these changes Mar 22, 2024
View reviewed changes
Copy link
Contributor

@charles-lunarg charles-lunarg left a comment

Choose a reason for hiding this comment

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

Tested it locally, and while the output is indeed garbage, I believe that is intentional.

I rebased on dev and added a missing --virtual-swapchain-skip-blit option in the vulkan USAGE markdown.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 154828.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3901 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3901 passed.

@MarkY-LunarG MarkY-LunarG force-pushed the virtual-swapchain-skip-blit branch from b630340 to c30965a Compare March 22, 2024 13:29
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 155122.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3904 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 155143.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3906 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3906 passed.

@marius-pelegrin-arm @charles-lunarg
Add option to skip blit of virtual swapchain
011297d 
Add an option during replay to skip blitting virtual swapchain to
real swapchain to remove the overhead of the blit. This overhead
is noticeable for microbenchmarks and impacts hardware counter
analysis.
The option is `--vssb` or `--virtual-swapchain-skip-blit`.

Change-Id: Idcda8e0c576f206de3286005d2ad970a74a05d01
@charles-lunarg charles-lunarg force-pushed the virtual-swapchain-skip-blit branch from 8f5ca0c to e8f6fa3 Compare March 29, 2024 16:01
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 159507.

@MarkY-LunarG @charles-lunarg
vulkan: Move swapchain options to single struct
96e44be 
Move all the swapchain options that are starting to show up into
a single structure that is set once for all swapchains.
@MarkY-LunarG @charles-lunarg
Fix code review comments
e8faea2
@charles-lunarg charles-lunarg force-pushed the virtual-swapchain-skip-blit branch from e8f6fa3 to e8faea2 Compare March 29, 2024 16:06
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 159510.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3946 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3946 passed.

@charles-lunarg charles-lunarg merged commit fa9b966 into LunarG:dev Mar 29, 2024
8 checks passed
@nickdriscoll-lunarg nickdriscoll-lunarg mentioned this pull request Apr 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrew-lunarg andrew-lunarg andrew-lunarg left review comments

@per-mathisen-arm per-mathisen-arm per-mathisen-arm left review comments

@charles-lunarg charles-lunarg charles-lunarg approved these changes

@MarkY-LunarG MarkY-LunarG Awaiting requested review from MarkY-LunarG

@bradgrantham-lunarg bradgrantham-lunarg Awaiting requested review from bradgrantham-lunarg

Assignees
No one assigned
Labels
P2 A high-priority code maintenance issue or a functional problem that is recoverable or not a crash.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@marius-pelegrin-arm @ci-tester-lunarg @bradgrantham-lunarg @per-mathisen-arm @MarkY-LunarG @lunarpapillo @charles-lunarg @andrew-lunarg