Closed Bug 1468108 Opened Last year Closed Last year

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

Categories

(Core :: Networking, defect, P2)

All
Unspecified
defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: mstange, Assigned: baku)

References

Details

(Whiteboard: [qf][necko-triaged])

Attachments

(1 file)

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.
Andrea, you appear to have written some of this code. Any idea how we should approach this?
Flags: needinfo?(amarchesini)
Whiteboard: [qf]
(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.
Flags: needinfo?(amarchesini)
Attached patch stream.patchSplinter Review
Valentin, feedback?
Assignee: nobody → amarchesini
Attachment #8985667 - Flags: review?(valentin.gosu)
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.
Attachment #8985667 - Flags: review?(valentin.gosu) → review+
Priority: -- → P2
Whiteboard: [qf] → [qf][necko-triaged]
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/61882e8a3cf3
Increase the buffer size and avoid extra runnables in NS_ReadInputStreamToBuffer, r=valentin
https://hg.mozilla.org/mozilla-central/rev/61882e8a3cf3
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Blocks: 1469290
You need to log in before you can comment on or make changes to this bug.