Closed
Bug 1384328
Opened 7 years ago
Closed 7 years ago
IAccessible2::get_relations is expensive
Categories
(Core :: Disability Access APIs, enhancement, P1)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: bugzilla, Assigned: bugzilla)
References
()
Details
(Whiteboard: aes+)
Attachments
(1 file)
(deleted),
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
There is a profile in the URL field.
Notice that when we filter on get_relations, we see that most of the parent process main thread activity is due to unmarshaling the IAccessibleRelation interfaces in the relations outparam.
I am working on obtaining a higher-resolution profile to see if we can get a better view.
Updated•7 years ago
|
Severity: normal → major
Priority: -- → P1
Assignee | ||
Comment 1•7 years ago
|
||
Some updates here:
I profiled this with xperf and was able to see where most of the time is being spent in the RPC service.
The service is spending most of its time maintaining its set of OIDs for pinging (ie, COM garbage collection).
Note that we don't really need pinging for proxies in the parent process because our content processes will never outlive our parent process.
I have tried a WIP patch that disables pinging when we know that we're destined for the parent process's main thread, but it is not working correctly in conjunction with handler marshaling (the correct flag is not being set by the standard marshaler).
My next attempt is going to be to manually set the SORF_NOPING flag in the marshaled OBJREF and then retesting with that.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
My test patch helps _a_lot_!
Perf is much better, although it is still pretty bad. The worst hangs are on sync TextChange events.
At least the remaining stuff is either controlled by us or by JAWS; I've taken the COM service out of the equation.
Assignee | ||
Comment 3•7 years ago
|
||
This patch ended up being simpler than I was expecting. It comprises of two changes:
1) We now unconditionally implement IMarshal. Previously we would only indicate that we implemented it if there was a valid handler.
2) The three IMarshal methods called during marshaling (GetUnmarshalClass, GetMarshalSizeMax, MarshalInterface) now call GetMarshalFlags when passing marshaling flags to the standard marshaler. GetMarshalFlags checks to see who the client is, and if that client is the parent main thread, it ORs MSHLFLAGS_NOPING into the marshal flags.
Attachment #8892540 -
Flags: review?(jmathies)
Updated•7 years ago
|
Attachment #8892540 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 4•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/71df96e65afad583532ff8351b1c1022324507c6
Bug 1384328: Marshal proxies destined for parent main thread with MSHLFLAGS_NOPING; r=jimm
Assignee | ||
Comment 5•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4ecd488495e695b3e3329e889f64b9c9e00d4b4
Bug 1384328: Follow-up: Fix some error handling problems exposed by 71df96e65afa; r=bustage
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/71df96e65afa
https://hg.mozilla.org/mozilla-central/rev/b4ecd488495e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•