1971470 - Pasting text on https://mbbill.github.io/JSC.js/demo/index.html is 3.5x slower in Firefox compared to Chrome

Status: NEW

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Mayank Bansal]

Attachment: sample text.js

Go to https://mbbill.github.io/JSC.js/demo/index.html
Let it load. May take a few seconds
Download the sample text to your local machine and copy-paste its contents on the page. DO NOT CLICK inside the page.

Firefox: https://share.firefox.dev/4jIlsW4 (25s)
Chrome: https://share.firefox.dev/4kEHWsv (7s)

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Masayuki,
Can you take a quick at the profile and see if there's any quick action or easy win?

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Well, I'm not sure whether it's related each other though, it stays with sleeping for IPC, the tree looks like:

+ mozilla::detail::ConditionVariableImpl::wait
  + mozilla::OffTheBooksCondVar::Wait (34%)
    + mozilla::TaskController::GetRunnableForMTTask
    + mozilla::TaskController::ProcessPendingMTTask
  + mozilla::detail::ConditionVariableImpl::wait (0%)
     <snip>
       + (anonymous namespace)::ClipboardDataSnapshotProxy::GetDataSync
         + mozilla::EditorBase::GetDataFromDataTransferOrClipboard::::operator()
           + mozilla::EditorBase::GetDataFromDataTransferOrClipboard
             + mozilla::EditorBase::GetDataFromDataTransferOrClipboard

So, it could be caused by a performance issue of clipboard access or creating the data transfer in the main process. Maybe, edgar may have some ideas.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

On the other hand, it stays 18% time in nsINode::ComputeIndexOf(). That's caused by here. Well, the selection change check is caused by mutations by the web app. We could optimize here to skip computing the offset if there is a quicker check in the specific case, but it may not improve the performance in usual apps. I mean, the fix may not be a potion for general cases.

Comment 5

[Edgar Chen [:edgar]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves) from comment #3)

So, it could be caused by a performance issue of clipboard access or creating the data transfer in the main process. Maybe, edgar may have some ideas.

ClipboardDataSnapshotProxy::GetDataSync() uses sync IPC, but from the profile, it only takes around 8.5ms. So it’s unlikely to contribute to performance issue too much.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thanks, Edgar.

I profiled it in my local environment, then, I got this. Then, the DOM part disappeared. Most time, it spends in RestyleManager::RestyleSiblingsStartingWith(nsIContent*).

Move this to the Layout.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

That doesn't match my profile at all? https://share.firefox.dev/4ncqYmK is focusing on RestyleManager, https://share.firefox.dev/44c55vd is focusing on SelectionChangeListener... Do you know why that difference might be?

Comment 8

[Emilio Cobos Álvarez [:emilio]]

In terms of restyling performance, this page is only using div + span, basically, so we only need to restyle one sibling, rather than arbitraryly many afterwards. This doesn't currently differentiate between both combinators (but that wouldn't quite be enough, we'd need some sort of count to deal with div + span + span...).

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't know... And about the SelectionChangeEventDispatcher, the cost is caused by nsINode::ComputeIndexOf() as I mentioned in comment 4. I tried to make it skip if the offset is start or end of its container, but I realized that the dispatcher anyway needs to compute the index. We could improve its performance with changing the cache in nsINode, but it might be overkill for a specific case.

Comment 10

[Mayank Bansal]

Latest: https://share.firefox.dev/4hYibTk (22s)