Bug 226330 – Double clicking on very large but visually empty DOM can be slow due to selection computation

• Component: CSS
• Status: NEW

[Liam DeBeasi]

Created attachment 429874 [details]
Code reproduction

This is a continuation of https://bugs.webkit.org/show_bug.cgi?id=222187 which was partially resolved in STP 125. When using Web Components inside of the Shadow DOM that have CSS Variables set on the host, there is a significant delay in response to clicks/touches. There were fixes in STP 125 that improved this issue, but I am still able to reproduce this issue in other ways.

Steps to reproduce:

1. Open code reproduction in STP 125.
2. Quickly click multiple times on the "Click Me" label (not the checkbox) and you should see Safari freeze up.
3. If you repeat step 2 while taking a Timeline sample, you should see the CPU usage spike significantly. In my tests I clicked about 4-5 times and saw the CPU spike to over 80% on average. 

Expected Behavior:

I would expect Safari to not freeze up when quickly clicking the label multiple times.

Actual Behavior:

Safari freezes up when quickly clicking the label multiple times.

Other Info:

- As noted in https://bugs.webkit.org/show_bug.cgi?id=222187, the example I used is inherently heavy in shadow trees to emphasize the issue. Please see https://github.com/ionic-team/ionic-framework/issues/22951 for more "real world" examples of this impacting applications.

[Alexey Proskuryakov]

Thank you for the report!

Is the performance still worse than pre-iOS 14, or is this about further optimization opportunities? Does this appear to be worse in Safari (STP) than in other browsers?

[Liam DeBeasi]

The performance of single clicking the checkbox in my example is good after https://bugs.webkit.org/show_bug.cgi?id=222187, but multiple clicks is where there are still issues. When doing multiple clicks, the performance in STP 125 is worse than in other browsers and worse than in pre-iOS/Safari 14.

[Radar WebKit Bug Importer]

<rdar://problem/78813010>

Hi guys, just wondered if there's been any movement on this?

Thanks,

[Philip Moore]

Be great to understand when this might get resolved, we’ve got a production app https://apps.apple.com/gb/app/mydrivetime-companion-app/id950711705 that’s being heavily impacted and stopping us from releasing a major update that our users need.

Many thanks

(In reply to Philip Moore from comment #5)
> Be great to understand when this might get resolved, we’ve got a production
> app https://apps.apple.com/gb/app/mydrivetime-companion-app/id950711705
> that’s being heavily impacted and stopping us from releasing a major update
> that our users need.
> 
> Many thanks

Hi Philip, by the fact this issue hasn't really moved forwards in over a year, I don't think anyone at webkit really cares about it. We had exactly the same issue last year and they've essentially ignored this thread. It seems we are expected to have write overly exuberant lazy loading component code to over come an issue that shouldn't be a problem to start with, as any other browser/device works fine, it's just anything Apple.

This is ridiculous, if I have an issue open for more than a 2 weeks my bosses complain, I think I would have been fired if I had a bug open for over a year.

[Liam DeBeasi]

Hey everyone,

While I know it is frustrating to not have your ticket resolved quickly, comments like the one in https://bugs.webkit.org/show_bug.cgi?id=226330#c6 are not super constructive.

Instead, comments that provide context around the impact of a bug (such as the comment in https://bugs.webkit.org/show_bug.cgi?id=226330#c5) are incredibly helpful. This context can be used to better prioritize issues.

Having worked on a large open source project (though not as large as a browser engine), I can say that it is not always possible to reply to every single issue/comment. Let’s try to keep this thread constructive and focus on identifying the impact this bug has on production apps/websites.

[alan]

I did a bit of profiling on this and it looks like (after the invalidation fix), this is simply about iterating over that massive amount of content when computing selection boundary. 
The following, simple markup triggers this issue:

<body>
Click Me<input type="checkbox"><br>
    <span></span>
    <span></span>
    <span></span>
    <span></span>
    <span></span>
repeat it for a few thousands of times
</body>

It looks like VisibleSelection needs a more efficient way to figure out the selection end on user action (or in general for that matter).

[Antti Koivisto]

Note that you can work around this issue by preventing the selection with "user-select:none".

[alan]

(In reply to zalan from comment #8)
> I did a bit of profiling on this and it looks like (after the invalidation
> fix), this is simply about iterating over that massive amount of content
> when computing selection boundary. 
> The following, simple markup triggers this issue:
> 
> <body>
> Click Me<input type="checkbox"><br>
>     <span></span>
>     <span></span>
>     <span></span>
>     <span></span>
>     <span></span>
> repeat it for a few thousands of times
> </body>
> 
> It looks like VisibleSelection needs a more efficient way to figure out the
> selection end on user action (or in general for that matter).
However if there's any "non-visually empty" content somewhere e.g.

<body>
Click Me<input type="checkbox"><br>
     <span></span>
     <span></span>
     <span>!!!some text!!!</span>
     <span></span>
     <span></span>
 repeat it for a few thousand times
</body>

I don't see any spinning anymore. So this is about iterating over massive amount of _visually empty_ content (see nextVisuallyDistinctCandidate()).

[Ryosuke Niwa]

Sounds like this is unrelated to shadow DOM. It's a good bug to address nonetheless.