Repository navigation
perf(agc): pool device staging buffers by size class - #1004
Open
Celegans12 wants to merge 1 commit into
Open
Celegans12 wants to merge 1 commit into
Celegans12 wants to merge 1 commit into
Conversation
The device-local tier of BufferPool only reused a buffer of the exact size and usage it was asked for. Partial storage-image detiles and retiles ask for a new size and the other transfer direction almost every time, so the tier sat at its budget and missed and evicted dozens of buffers per frame, and the vkFreeMemory calls on the release thread held up submits and allocations on the recording threads. Device-tier requests of 1 MiB and more now round up to an eighth of their octave, the tier's buffers are created for storage and both transfer directions, and a request takes the smallest retained buffer from its own class up to twice its size. Host tiers, the budget and eviction are unchanged. agc_buffer_pool covers the classes, the fit and its bound, reuse only after release, the budget and the unchanged lists.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
9 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The device-local tier of
BufferPoolonly reused a buffer of the exact same size and usage. Partial storage-image detiles and retiles ask for a new size and the other transfer direction almost every time, so in PPSA26344 the tier sits at its 512 MiB budget and misses and evicts ~77 buffers per frame. The release thread then spends ~38 ms per frame invkFreeMemory, which on NVIDIA also holds upvkQueueSubmit(mean ~0.7 ms).STORAGE | TRANSFER_SRC | TRANSFER_DSTadded (BufferPool::Usage), so uploads, write-backs and staging shadows share a list;SHADER_DEVICE_ADDRESSandINDIRECTrequests keep their own.Takeserves a request from the smallest retained buffer of its class up to twice its size; the buffer returns to its own class. Callers bind and copy explicit sizes, as with the small classes.APS5_NO_BUFFER_CLASSES=1still matches exact sizes.Resources.cppconflict is textual, so whichever lands second needs a small rebase.Tested
mainab9437e: configure-DBUILD_TESTING=ON, full build,--target libs,ctest:ctest --output-on-failure)VK_DRIVER_FILES): 367/367 pass;agc_driver_flat_storeis left out there because it takes several minutes on Windows llvmpipeagc_buffer_pool(mock device): size classes, shared directions, the 2x fit, reuse only after release, budget. Fails on main.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)