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

Batched/Linear uploads #56

Merged
merged 6 commits into from
Jan 18, 2024
Merged

Batched/Linear uploads #56

merged 6 commits into from
Jan 18, 2024

Conversation

Fuzzyzilla
Copy link
Contributor

@Fuzzyzilla Fuzzyzilla commented Jan 16, 2024

Replaces the allocate-as-needed logic with bulk linear allocations for both image create, image update, and mesh upload.

  • Doubles the framerate over master when compared using a modified demo_app.rs with PresentMode::Mailbox and a few spare swapchain images, even using 1% less of my CPU while doing so. It also starts up a sixteenth of a second faster, clearly the largest benefit here :P

  • Image uploads are now batched per-frame and only synchronize once. Should help with stutter on UIs that load/modify fonts or images frequently (Scrubbing the font size slider on the demo_app's FontBook is a good example).

    • Could further improve this by not synchronizing the host at all, leaving all sync to the device.
  • Mesh uploads now only memmap the buffers twice-per-frame instead of twice-per-mesh.

    • With a dependency on bytemuck this could be further reduced to a single memmap per frame, but it would require a private dependency addition and a public egui feature flag change for a somewhat trivial perf boost, and I'm not sure the policy on that (could also be achieved with some fairly trivial unsafe ops).
  • It should use substantially less memory during these operations, but I do not have a convenient way to measure. On current master, image creation/deltas involve 4 in-memory copies, reduced here to 2.

I have further optimizations in mind, namely deduplicating command buffer commands, as there are currently ~250 commands for 20 meshes which could be greatly reduced. Always unsure on whether I should bundle the changes into one PR since they're a bit disjoint -- please advise!

@hakolao
Copy link
Owner

hakolao commented Jan 16, 2024

Always unsure on whether I should bundle the changes into one PR since they're a bit disjoint -- please advise!

The smaller the PR the easier it is to review and less chance for mistakes. So Separate PRs 👍

@hakolao
Copy link
Owner

hakolao commented Jan 16, 2024

This one is also appreciated! Don't see anything blocking the merge other than lints & merge conflicts

@Fuzzyzilla
Copy link
Contributor Author

Alright, merged and fixed up! Did a few reads over and I'm happy enough with it to open this up. I hope I did alright with the merge, still learning git.

Doubles the framerate over master when compared using a modified demo_app.rs with PresentMode::Mailbox and a few spare swapchain images,

I should not be allowed to benchmark/write PR messages at 2am. The improvements are much more modest at ~25% improvement in frametimes. The other improvements are still true - I just have no idea how i got that number lol.

@Fuzzyzilla Fuzzyzilla marked this pull request as ready for review January 16, 2024 22:49
Copy link
Owner

@hakolao hakolao left a comment

Choose a reason for hiding this comment

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

Looking fine to me at this point and works at my end

@hakolao hakolao merged commit 8794e7d into hakolao:master Jan 18, 2024
4 checks passed
Letronix624 pushed a commit to Letronix624/egui_winit_vulkano that referenced this pull request Jan 20, 2024
* Batch upload images, skip 2 copies per image

* Batch upload meshes, map only twice.

* lints

* Integrate packed fonts

* Lints again, oops
Letronix624 pushed a commit to Letronix624/egui_winit_vulkano that referenced this pull request Feb 7, 2024
* Batch upload images, skip 2 copies per image

* Batch upload meshes, map only twice.

* lints

* Integrate packed fonts

* Lints again, oops
Letronix624 pushed a commit to Letronix624/egui_winit_vulkano that referenced this pull request Feb 12, 2024
* Batch upload images, skip 2 copies per image

* Batch upload meshes, map only twice.

* lints

* Integrate packed fonts

* Lints again, oops
Letronix624 pushed a commit to Letronix624/egui_winit_vulkano that referenced this pull request Mar 17, 2024
* Batch upload images, skip 2 copies per image

* Batch upload meshes, map only twice.

* lints

* Integrate packed fonts

* Lints again, oops
Letronix624 pushed a commit to Letronix624/egui_winit_vulkano that referenced this pull request Mar 23, 2024
* Batch upload images, skip 2 copies per image

* Batch upload meshes, map only twice.

* lints

* Integrate packed fonts

* Lints again, oops
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