[PATCH 5/5] dxgi: Implement adapter video memory budget change notification.

[Zebediah Figura]

On 5/20/22 02:15, Zhiyi Zhang wrote:
> 
> 
> On 5/20/22 15:05, Zhiyi Zhang wrote:
>>
>> On 5/20/22 14:16, Zebediah Figura wrote:
>>> On 5/18/22 01:49, Zhiyi Zhang wrote:
>>>> @@ -1046,6 +1060,135 @@ HRESULT CDECL wined3d_adapter_get_video_memory_info(const struct wined3d_adapter
>>>>        return WINED3D_OK;
>>>>    }
>>>>    +static DWORD CALLBACK notification_thread_func(void *stop_event)
>>>> +{
>>>> +    struct wined3d_adapter_budget_change_notification *notification;
>>>> +    struct wined3d_video_memory_info info;
>>>> +    HRESULT hr;
>>>> +
>>>> +    while (TRUE)
>>>> +    {
>>>> +        wined3d_mutex_lock();
>>>> +        LIST_FOR_EACH_ENTRY(notification, &adapter_budget_change_notifications,
>>>> +                struct wined3d_adapter_budget_change_notification, entry)
>>>> +        {
>>>> +            hr = wined3d_adapter_get_video_memory_info(notification->adapter, 0,
>>>> +                    WINED3D_MEMORY_SEGMENT_GROUP_LOCAL, &info);
>>>> +            if (SUCCEEDED(hr) && info.budget != notification->last_local_budget)
>>>> +            {
>>>> +                notification->last_local_budget = info.budget;
>>>> +                SetEvent(notification->event);
>>>> +                continue;
>>>> +            }
>>>> +
>>>> +            hr = wined3d_adapter_get_video_memory_info(notification->adapter, 0,
>>>> +                    WINED3D_MEMORY_SEGMENT_GROUP_NON_LOCAL, &info);
>>>> +            if (SUCCEEDED(hr) && info.budget != notification->last_non_local_budget)
>>>> +            {
>>>> +                notification->last_non_local_budget = info.budget;
>>>> +                SetEvent(notification->event);
>>>> +            }
>>>> +        }
>>>> +        wined3d_mutex_unlock();
>>>> +
>>>> +        if (WaitForSingleObject(stop_event, 1000) == WAIT_OBJECT_0)
>>>> +            break;
>>> The test also waits for 1 second, which seems like it'd be liable to fail intermittently, if I read this correctly. Should we increase the polling interval here? (Or reduce it in the test?)
>> I can probably reduce the timeout in the test. It was picked rather randomly and didn't mean the test would fail intermittently.
>>
> 
> Are you worried that the event may not be fired within 1 second? If an adapter has any memory budget, the event will be immediately fired due to
> notification->last_local_budget and notification->last_non_local_budget being initialized to zero. The test's purpose is to test the event should be
> fired immediately, so the 1 second wait doesn't really matter.

Ah, indeed.

Although that raises another question—is every subscriber supposed to be 
notified immediately? And if so, that won't happen currently, not 
without alerting the thread that a new subscriber has been added. If the 
notification doesn't need to be immediate there's not a problem though...

> 
> Thanks,
> Zhiyi
> 
>>>> +    }
>>>> +
>>>> +    return TRUE;
>>>> +}
>>>> +
>>>> +HRESULT CDECL wined3d_adapter_register_budget_change_notification(const struct wined3d_adapter *adapter,
>>>> +        HANDLE event, DWORD *cookie)
>>>> +{
>>>> +    static DWORD cookie_counter;
>>>> +    static BOOL wrapped;
>>>> +    struct wined3d_adapter_budget_change_notification *notification, *new_notification;
>>>> +    HANDLE thread = NULL;
>>>> +    BOOL found = FALSE;
>>>> +
>>>> +    new_notification = heap_alloc_zero(sizeof(*new_notification));
>>>> +    if (!new_notification)
>>>> +        return E_OUTOFMEMORY;
>>>> +
>>>> +    wined3d_mutex_lock();
>>>> +    new_notification->adapter = adapter;
>>>> +    new_notification->event = event;
>>>> +    new_notification->cookie = cookie_counter++;
>>>> +    if (cookie_counter < new_notification->cookie)
>>>> +        wrapped = TRUE;
>>>> +    if (wrapped)
>>>> +    {
>>>> +        while (TRUE)
>>>> +        {
>>>> +            LIST_FOR_EACH_ENTRY(notification, &adapter_budget_change_notifications,
>>>> +                    struct wined3d_adapter_budget_change_notification, entry)
>>>> +            {
>>>> +                if (notification->cookie == new_notification->cookie)
>>>> +                {
>>>> +                    found = TRUE;
>>>> +                    break;
>>>> +                }
>>>> +            }
>>>> +
>>>> +            if (!found)
>>>> +                break;
>>>> +
>>>> +            new_notification->cookie = cookie_counter++;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    *cookie = new_notification->cookie;
>>>> +    list_add_head(&adapter_budget_change_notifications, &new_notification->entry);
>>>> +
>>>> +    if (!notification_thread)
>>>> +    {
>>>> +        notification_thread_stop_event = CreateEventW(0, FALSE, FALSE, NULL);
>>>> +        thread = CreateThread(NULL, 0, notification_thread_func, notification_thread_stop_event,
>>>> +                CREATE_SUSPENDED, NULL);
>>>> +        notification_thread = thread;
>>>> +    }
>>>> +    wined3d_mutex_unlock();
>>>> +    if (thread)
>>>> +        ResumeThread(thread);
>>> What's the reason for creating the thread suspended?
>> Because the thread also calls wined3d_mutex_lock/unlock().

Right, but that shouldn't require suspending it. It just means that if 
the thread starts executing, it'll wait on the wined3d mutex until we 
release it subsequently.

>>
>>>> +    return WINED3D_OK;
>>>> +}
>>
>