1414497 - [e10s a11y] Reduce cross-process QueryInterface using IMultiQI

Status: RESOLVED WONTFIX

(Core :: Disability Access APIs, enhancement, P2)

Description

[James Teh [:Jamie]]

Now that virtual buffers have to render across processes, we want to eliminate as many cross-process calls as possible. This includes QueryInterface calls, since buffers query for several interfaces on every node they visit. One way we can do this is tweaking the handler to use IMultiQI, an interface implemented by COM which allows a caller to query for multiple interfaces in a single cross-process call:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683863(v=vs.85).aspx

Note that this still requires us to make one extra cross-process call after we receive the initial interface. Ideally, we would eliminate even this and instead include the interface pointers in the handler payload. Unfortunately, this is proving to be rather tricky to implement. We should still pursue this in future, but this first pass with IMultiQI should at least allow us to determine whether reducing these calls makes much difference.

Comment 1

[James Teh [:Jamie]]

Try build:
https://tools.taskcluster.net/groups/bFHTg0vwSROhqojcaEjrrg/tasks/VwHKA60JTVCha83ttYLOJw/runs/0/artifacts

Marco, it'd be great if you could give this a try and see if you notice any performance difference. Surprisingly, I wasn't seeing any notable difference with local builds, which just doesn't make any theoretical sense to me. (Sorry about the previous try build on IRC; bad rebase. :) )

Comment 2

[James Teh [:Jamie]]

Here's another try build with some further fixes (though I'm still not sure whether there's a perf improvement when testing local builds):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=63d405c76754d02cf79d6723ce031ade86cbf53c

Comment 3

[James Teh [:Jamie]]

Aaron, a couple of questions related to this:

1. I need to track E_NOINTERFACE results from QueryMultipleInterfaces, since some timing tests revealed that COM caches interfaces but does *not* cache E_NOINTERFACE results. I tried using an nsTArray, but the build fails when I do that because nsTArray.cpp isn't included in the handler anywhere. I wasn't sure what the implications of trying to use that in a non-Mozilla process were, so I used an std::vector. Is that going to be a problem, and if so, what would you recommend?
2. If QueryHandlerInterface returns E_NOINTERFACE, Handler::InternalQueryInterface falls back to querying the proxy. That normally makes perfect sense, but in this case, I know that the proxy doesn't have the interface and I don't want it wasting a cross-proc call. I thought I could invent a custom HRESULT (HANDLER_E_NOINTERFACE_CACHED?) for this purpose, which InternalQI would catch and immediately return E_NOINTERFACE. Do you have any better ideas?

Comment 4

[Marco Zehe (:MarcoZ)]

Jamie, the latest try build is about a second faster on my machine than regular nightly, e. g. under 7 seconds for World War I instead of 8. Note, however, that I am not using an NVDA build yet with the IaccessibleHypertext2 stuff with multiple hyperlinks. But anyway, this seems to make things more fluid yet again.

Comment 5

[James Teh [:Jamie]]

Attachment: Bug 1414497 part 1: AccessibleHandler: Use IMultiQI to cache interfaces the client is likely to request.

Now that virtual buffers have to render across processes, we want to eliminate as many cross-process calls as possible.
This includes QueryInterface calls, since buffers query for several interfaces on every node they visit.
We do this using IMultiQI, an interface implemented by COM which allows a caller to query for multiple interfaces in a single cross-process call.

Review commit: https://reviewboard.mozilla.org/r/197146/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197146/

Comment 6

[James Teh [:Jamie]]

Attachment: Bug 1414497 part 2: Accessible HandlerProvider: Cache interceptors needed for IMultiQI in the main thread.

The handler will query for a bunch of interfaces using IMultiQI.
However, creating each interceptor in the MTA requires a separate cross-thread call to QI in the STA.
Instead of incurring cross-thread overhead for each call, fetch all the interceptors in the main thread when building the payload.

Review commit: https://reviewboard.mozilla.org/r/197148/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197148/

Comment 7

[James Teh [:Jamie]]

Attachment: Bug 1414497 part 3: AccessibleHandler: Cache E_NOINTERFACE when caching interfaces.

The COM proxy manager caches interfaces, but it does not cache E_NOINTERFACE results.
If we get E_NOINTERFACE once, we're going to keep getting it, so there's no point in wasting a cross-process call.
Therefore, cache E_NOINTERFACE results ourselves.

Review commit: https://reviewboard.mozilla.org/r/197150/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197150/

Comment 8

[James Teh [:Jamie]]

I thought it might be useful to see where I'm going with this. However, part 3 is definitely not the final patch. (Unfortunately, it doesn't seem you can do a feedback request with mozreview instead of a review request.) Right now, it just returns E_FAIL instead of E_NOINTERFACE in the cached E_NOINTERFACE case. See comment 3 for questions about this.

Comment 9

[James Teh [:Jamie]]

Closing in favour of bug 1416986, since that approach is now working and provides even better performance.