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

[Henri Verbeet]

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.)