On 1/31/22 16:44, Henri Verbeet wrote:
On Mon, 31 Jan 2022 at 19:02, Zebediah Figura zfigura@codeweavers.com wrote:
On 1/31/22 11:33, Henri Verbeet wrote:
On Mon, 31 Jan 2022 at 18:22, Zebediah Figura zfigura@codeweavers.com wrote:
On 1/31/22 08:11, Henri Verbeet wrote:
On Mon, 31 Jan 2022 at 04:05, Zebediah Figura zfigura@codeweavers.com wrote:
The ultimate idea being to set a cap on the amount of persistent BO memory, in which case we need to arbitrarily mark BOs as persistent.
Could the "arbitrary" part be avoided? In particular, what would prevent us from unmapping less recently used mappings when we run into the cap?
Nothing, really. We could relatively easily use an LRU list, or periodically garbage-collect unused mappings, for two possibilities.
I think it's a bit orthogonal to this patch, though. No matter what criteria we use to determine whether a mapping is persistent in wined3d_bo_*_map(), or to possibly unmap it later, we'll at least want some of these users of bo->persistent.
I'm also inclined to argue there's no reason not to accept 4/4 now, and later (assuming we need to) implement a more complex and optimized scheme on top of that. Although it could probably do with a bit more factored out into the "adjust_mapped_memory" helpers.
To some extent, yes. However, the follow-up question then becomes what the "persistent" field really means. Or put a different way, under what circumstances should we use "bo->persistent" instead of "bo->map_ptr"?
Note that we use "bo->map_ptr" inconsistently between the OpenGL and Vulkan backends—the OpenGL backend only sets it for persistent mappings, whereas the Vulkan backend sets it for all mappings. This patch standardizes on the latter behaviour.
The reason we want this is basically patch 2/4. Namely, if the BO is mapped, then wined3d_device_context_map() can return its map pointer to the caller (assuming we just got it from a DISCARD map). If it is mapped persistently, we can also store that map pointer and use it for subsequent NOOVERWRITE maps.
The concern I have is that if we're using "persistent" in the sense the the bo is never going to be unmapped, that essentially precludes futures schemes where we do potentially unmap these. On the other hand, if these aren't all that persistent in practice, we only care whether the bo is currently mapped, but then also need to make sure we can invalidate e.g. client mappings when needed. (And note that in principle a bo can be currently mapped because a different bo in the same slab or chunk is currently mapped, regardless of what wined3d_map_persistent() returns.)
Right, took me long enough to realize that's what you were getting at...
I guess it ultimately does depend on how we unmap, but no matter how we do it, we probably want the client thread to be managing it, and in that case the "persistent" field would be redundant.
I'll try to restructure things so that the client thread is managing the existing persistent mapping logic.