Bug 237782 – Delay responsiveness checks for the Network Process until it has finished initialization

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Delay responsiveness checks for the Network Process until it has finished initialization.

Network process initialization can be slow but we have evidence that it is true truly hung since we see network process terminations right as the network process is initiating the WebProcess connection later on.

[Chris Dumez]

<rdar://88226412>

[Chris Dumez]

Created attachment 454489 [details]
Patch

[Geoffrey Garen]

Comment on attachment 454489 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=454489&action=review

r=me

> Source/WebKit/ChangeLog:10
> +        Network process initialization can be slow but we have evidence that it is true truly

true truly -> not truly?

> Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:371
> +        m_delayedResponsivenessCheck = useLazyStop;

When m_didBeginResponsivenessChecks is true, the *first* useLazyStop value wins, and all subsequent values are ignored so long as the timer is still active.

When m_didBeginResponsivenessChecks is false, the *last* useLazyStop value wins, and all previous values are overwritten.

Is this something we should care about? -- Maybe we don't care because useLazyStop is just a performance optimization for the case where we do schedule the timer?

[Chris Dumez]

Comment on attachment 454489 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=454489&action=review

>> Source/WebKit/ChangeLog:10
>> +        Network process initialization can be slow but we have evidence that it is true truly
> 
> true truly -> not truly?

Oops :)

>> Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:371
>> +        m_delayedResponsivenessCheck = useLazyStop;
> 
> When m_didBeginResponsivenessChecks is true, the *first* useLazyStop value wins, and all subsequent values are ignored so long as the timer is still active.
> 
> When m_didBeginResponsivenessChecks is false, the *last* useLazyStop value wins, and all previous values are overwritten.
> 
> Is this something we should care about? -- Maybe we don't care because useLazyStop is just a performance optimization for the case where we do schedule the timer?

Right, it is an optimization for repeated start calls so I don't think it is critical here. That said, I think you're right it's good to be consistent so I'll fix.

[Chris Dumez]

Created attachment 454495 [details]
Patch

[Sihui Liu]

Do we know the cause of the slowness during initialization (I guess not all crashes point to random call stack )? If we do, could you add comment about the cause to the radar or FIXME in the code so that maybe we can make optimization in the future?

[Chris Dumez]

(In reply to Sihui Liu from comment #6)
> Do we know the cause of the slowness during initialization (I guess not all
> crashes point to random call stack )? If we do, could you add comment about
> the cause to the radar or FIXME in the code so that maybe we can make
> optimization in the future?

The comments are already in the radar and as far as I can tell, it is already the same stack we're aware of.

[EWS]

Committed r291182 (248341@main): <https://commits.webkit.org/248341@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454495 [details].