1645473 - Retrieving virtual desktop info during session store performs synchronous IPC on the parent process main thread

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P2)

Description

[(No longer employed by Mozilla) Aaron Klotz]

Resolving the IVirtualDesktopManager interface is expensive because it needs to connect to the shell over COM. Not only does this initiate IPC between Firefox and the shell, but because COM does a bunch of bookkeeping about the connection, there is also IPC with a Windows background service.

Once we have that interface, we then call IVirtualDesktopManager::GetWindowDesktopId which is another synchronous IPC call.

From what I can tell, this code runs every time that session store refreshes its state, so fairly often.

So:

• CoCreateInstance, IServiceProvider::QueryService, and IUnknown::QueryInterface are both synchronous and expensive across process boundaries.
• The final Release of the remote interface is also synchronous and expensive, as it also needs to cross process boundaries.
• The method call itself is slightly cheaper, however it is still synchronous IPC on our parent process main thread.

What should we do about it?

• There is a CLSID_VirtualDesktopManager that is implemented as an in-process server. Can we use that instead and avoid IPC entirely?
• If not, at the very least we should create a shared IVirtualDesktopManager and retain it for the lifetime of our process.
• We should think about making anything that manipulates or touches that interface asynchronous and running it on a background thread in the multithreaded apartment;
• We should look further into what types of notifications are sent to a window when it is moved to a different virtual desktop. Since querying is expensive, perhaps we should only do that after we know we've been moved. As a hypothetical example, suppose that WM_MOVE is sent when a window is moved to a different virtual desktop. There would be no point in re-querying for virtual desktop information unless we have seen the move message first.

Comment 1

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 890125

Comment 2

[Kim Moir [:kmoir] ET]

Jim Can we can a priority and severity on this bug?

Comment 3

[Greg Stoll :gstoll]

Bug 1640852 and bug 1833841 have addressed the issues here - we cache the IVirtualDesktopManager after creation, and we no longer call it on the main thread.