1468108 - HttpChannelParent::OnDataAvailable can jank the main thread when NS_ReadInputStreamToBuffer blocks on a lock (for buffer stealing?)

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Markus Stange [:mstange]]

See this profile: https://perfht.ml/2HFmQGW

nsHttpChannel::OnDataAvailable spends all most half of its time waiting on a lock inside NS_ReadInputStreamToBuffer. Based on reading the code, I think this happening inside the writer->StealBuffer() call.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Andrea, you appear to have written some of this code. Any idea how we should approach this?

Comment 2

[Andrea Marchesini [:baku]]

(In reply to Valentin Gosu [:valentin] from comment #1)
> Andrea, you appear to have written some of this code. Any idea how we should
> approach this?

Yes, I wrote this code, and I suspect the lock doesn't happen in writer->StealBuffer() but, more likely here:

https://searchfox.org/mozilla-central/source/netwerk/base/nsNetUtil.cpp#1658-1664

Here we have monitor.wait() because the reading of the stream happens on a IO thread, and the main-thread needs to wait the ending of that operation in order to return the content as a string.

Something we can do to improve this code is here:

https://searchfox.org/mozilla-central/source/netwerk/base/nsNetUtil.cpp#1700

What I would do is:

1. maybe read more than 4096 bytes.
2. try consecutive ReadSegments() instead of dispatching new runnables.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: stream.patch

Valentin, feedback?

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8985667 [details] [diff] [review]
stream.patch

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

Looks good to me assuming try is all green.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/61882e8a3cf3
Increase the buffer size and avoid extra runnables in NS_ReadInputStreamToBuffer, r=valentin

Comment 6

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/61882e8a3cf3