[PATCH 3/4] wined3d: Keep track of whether a BO is persistently mapped in a per-BO field.

[Zebediah Figura]

On 1/31/22 16:44, Henri Verbeet wrote:
> On Mon, 31 Jan 2022 at 19:02, Zebediah Figura <zfigura at codeweavers.com> wrote:
>> On 1/31/22 11:33, Henri Verbeet wrote:
>>> On Mon, 31 Jan 2022 at 18:22, Zebediah Figura <zfigura at codeweavers.com> wrote:
>>>> On 1/31/22 08:11, Henri Verbeet wrote:
>>>>> On Mon, 31 Jan 2022 at 04:05, Zebediah Figura <zfigura at 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.