[Mac] After initial page load, first VoiceOver interaction with HTML content still very painfully slow

NEW
Unassigned

Status

()

Core
Disability Access APIs
6 years ago
12 days ago

People

(Reporter: MarcoZ, Unassigned)

Tracking

(Blocks: 1 bug, {perf})

Trunk
x86_64
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sps][leave open], URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
STR:
1. With VoiceOver on, load http://www.heise.de/mac-and-i
2. Wait until VoiceOver announces "HTML content".
3. Press CTRL+Option+Shift+DownArrow to start interaction and start reading the content.

Result: On my 2.7 gHz Core I5 MacBook pro, VoiceOver will now say "Firefox Nightly busy", and it will not come back with any other reaction for 50 (that's fifty) seconds. Then, it will say "Firefox Nightly Ready".

Smaller pages like the Firefox start page are faster, the more content on a page, the slower it gets.
How long should I wait between 1 and 2?
OK I think this is a relevant profile: http://is.gd/lpj44g
Yes our profiles are similar.
When I run this on debug, one of the stack trace I find taking some time (not that much, but still) is bug 768997. It is a regression.

Updated

6 years ago
Depends on: 768997
(In reply to Hub Figuiere [:hub] from comment #5)
> When I run this on debug, one of the stack trace I find taking some time
> (not that much, but still) is bug 768997. It is a regression.

profiling debug builds is basically pointless.  That code is only in debug builds which aren't the issue we care about here, so its not relevent.
No longer depends on: 768997
Marco, is it a debug build or a release build you use?
(Reporter)

Comment 8

6 years ago
Release, regular nightly. I usually test with that to get as close to the end user experience as possible. Unless I note otherwise, you can always assume I run a release build.

Updated

6 years ago
Whiteboard: [sps]

Updated

6 years ago
Duplicate of this bug: 782700
Created attachment 652865 [details] [diff] [review]
Implement accessibilityArrayAttributeCount for mozPaneAccessible. r=
Comment on attachment 652865 [details] [diff] [review]
Implement accessibilityArrayAttributeCount for mozPaneAccessible. r=

This one remove some "noise" in the performance analysis. A trivial performance improvement.
Attachment #652865 - Flags: review?(surkov.alexander)
Comment on attachment 652865 [details] [diff] [review]
Implement accessibilityArrayAttributeCount for mozPaneAccessible. r=

> 
>+- (NSUInteger)accessibilityArrayAttributeCount:(NSString *)attribute

nit type* name

>+  if([attribute isEqualToString:NSAccessibilityChildrenAttribute]) {
>+    return mGeckoAccessible->ChildCount() ? 1 : 0;

I think you could just use FirstChild() here which would be a bit faster.

>+  }

nit, no braces
I don't think FirstChild() would be faster. ChildCount() just call mChildren.Length().
Created attachment 652947 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible. r=

Updated

6 years ago
Attachment #652865 - Attachment is obsolete: true
Attachment #652865 - Flags: review?(surkov.alexander)
Comment on attachment 652947 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible. r=

Dealt with the 'nit from Trevor.
Attachment #652947 - Flags: review?(surkov.alexander)
(In reply to Hub Figuiere [:hub] from comment #13)
> I don't think FirstChild() would be faster. ChildCount() just call
> mChildren.Length().

true, but its virtual, FirsChild isn't

Comment 17

6 years ago
Comment on attachment 652947 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible. r=

Review of attachment 652947 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with my and Trevor's comments addressed

::: accessible/src/mac/mozActionElements.mm
@@ +342,5 @@
> +{
> +  if (!mGeckoAccessible)
> +    return 0;
> +
> +  if([attribute isEqualToString:NSAccessibilityChildrenAttribute])

nit: space after if

please add a comment that this is a performance trick because mozPaneAccessible doesn't keep its child in the cached children array
Attachment #652947 - Flags: review?(surkov.alexander) → review+
(In reply to Trevor Saunders (:tbsaunde) from comment #16)
> (In reply to Hub Figuiere [:hub] from comment #13)
> > I don't think FirstChild() would be faster. ChildCount() just call
> > mChildren.Length().
> 
> true, but its virtual, FirsChild isn't

FirstChild() calls GetChildAt() (inline)
GetChildAt() (virtual) calls mChildren.SafeElementAt() which call nsTArray<>::Length().
(In reply to Hub Figuiere [:hub] from comment #18)
> (In reply to Trevor Saunders (:tbsaunde) from comment #16)
> > (In reply to Hub Figuiere [:hub] from comment #13)
> > > I don't think FirstChild() would be faster. ChildCount() just call
> > > mChildren.Length().
> > 
> > true, but its virtual, FirsChild isn't
> 
> FirstChild() calls GetChildAt() (inline)
> GetChildAt() (virtual) calls mChildren.SafeElementAt() which call
> nsTArray<>::Length().

err, yeah, I should have said ContentChildCount(), but I'm not absolutely sure I think that's a good idea to use here (we should clarify exactly when we support using that).
Yeah maybe we can use ContentChildCount() as I believe we'll only encounter that kind of child (it is not a treeview or something)
Created attachment 653520 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible.

Updated

6 years ago
Attachment #652947 - Attachment is obsolete: true
Comment on attachment 653520 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible.

use ContentChildCount() instead of ChildCount. Other nits addressed.
Attachment #653520 - Flags: review?(trev.saunders)

Comment 23

6 years ago
Comment on attachment 653520 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible.

Review of attachment 653520 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/mac/mozActionElements.mm
@@ +338,5 @@
>  
>  @implementation mozPaneAccessible
>  
> +// By default this calls -[[mozAccessible children] count].
> +// Since we don't cache mChildren find a faster way.

it makes sense to move this comment to the code it's about, i.e. before that if statement. I assume this method might be used to speed up other properties as well.

@@ +345,5 @@
> +  if (!mGeckoAccessible)
> +    return 0;
> +
> +  if ([attribute isEqualToString:NSAccessibilityChildrenAttribute])
> +    return mGeckoAccessible->ContentChildCount() ? 1 : 0;

I wouldn't use ContentChildCount() until it's a real bottleneck.
Created attachment 653752 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible.

Updated

6 years ago
Attachment #653520 - Attachment is obsolete: true
Attachment #653520 - Flags: review?(trev.saunders)
Comment on attachment 653752 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible.

Using ChildCount() again.
Attachment #653752 - Flags: review?(trev.saunders)
Comment on attachment 653752 [details] [diff] [review]
Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible.

># HG changeset patch
># User Hubert Figuière <hfiguiere@mozilla.com>
># Date 1342052959 25200
># Node ID 072e6d50c5c30910919301d0051fb69ac14bd63e
># Parent  75ed4a4cc1024e35dd3577e52d818face035c2b7
>Bug 772060 - Part 1: Implement accessibilityArrayAttributeCount for mozPaneAccessible. r=surkov,tbsaunde
>
>diff --git a/accessible/src/mac/mozActionElements.mm b/accessible/src/mac/mozActionElements.mm
>--- a/accessible/src/mac/mozActionElements.mm
>+++ b/accessible/src/mac/mozActionElements.mm
>@@ -333,16 +333,29 @@ enum CheckboxValue {
>   [mTabs release];
>   mTabs = nil;
> }
> 
> @end
> 
> @implementation mozPaneAccessible
> 
>+- (NSUInteger)accessibilityArrayAttributeCount:(NSString*)attribute
>+{
>+  if (!mGeckoAccessible)
>+    return 0;
>+
>+  // By default this calls -[[mozAccessible children] count].
>+  // Since we don't cache mChildren find a faster way.

find a faster way is a little funny phraseing to me, how about "so this is faster"
Attachment #653752 - Flags: review?(trev.saunders) → review+

Updated

6 years ago
Whiteboard: [sps] → [sps][leave open]

Updated

6 years ago
Attachment #653752 - Flags: checkin+
Backed out because it landed on a burning tree

https://hg.mozilla.org/integration/mozilla-inbound/rev/46365c62935c

Comment 31

13 days ago
Marcoz, is this still an issue?  It is the last open bug blocking meta bug 454202
Flags: needinfo?(mzehe)
(Reporter)

Comment 32

13 days ago
Yes, and there are probably many more since this got filed. Mac isn't a priority for us right now, so I wouldn't count on that dependency tree to be accurate.
Flags: needinfo?(mzehe)
You need to log in before you can comment on or make changes to this bug.