Closed Bug 1426578 Opened 6 years ago Closed 6 years ago

Offload MediaCacheStream::Init/InitAsClone to another thread

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

Product:

Component:

Type:

enhancement

Priority:

P3

Severity:

normal

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla59

Tracking Flags:

Tracking

Status

firefox59

---

fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(5 files)

Bug 1426578. P1 - tweak the constructor and init functions of ChannelMediaResource/MediaCacheStream. 6 years ago JW Wang [:jwwang] [:jw_wang] 59 bytes, text/x-review-board-request	bechen : review+ mozbugz : review+	Details
Bug 1426578. P2 - always access MediaCache::mStreams while the cache monitor is held. 6 years ago JW Wang [:jwwang] [:jw_wang] 59 bytes, text/x-review-board-request	bechen : review+ mozbugz : review+	Details
Bug 1426578. P3 - make MediaCacheStream::InitAsClone() infallible. 6 years ago JW Wang [:jwwang] [:jw_wang] 59 bytes, text/x-review-board-request	bechen : review+ mozbugz : review+	Details
Bug 1426578. P4 - offload InitAsClone() to another thread. 6 years ago JW Wang [:jwwang] [:jw_wang] 59 bytes, text/x-review-board-request	bechen : review+ mozbugz : review+	Details
Bug 1426578. P5 - offload Init() to another thread. 6 years ago JW Wang [:jwwang] [:jw_wang] 59 bytes, text/x-review-board-request	bechen : review+ mozbugz : review+	Details

JW Wang [:jwwang] [:jw_wang]

Assignee

Description

•

6 years ago

We are almost there! Only a few functions left that need to be moved to a non-main thread.

JW Wang [:jwwang] [:jw_wang]

Assignee

Updated

•

6 years ago

Assignee: nobody → jwwang

Blocks: 1369263

Priority: -- → P3

Comment hidden (mozreview-request)

We want to init as many members as possible before taking the cache monitor.
This makes it easier to move part of the init functions to another thread.

Review commit: https://reviewboard.mozilla.org/r/208990/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208990/

Comment hidden (mozreview-request)

Note we offload MediaCache::CloseStreamsForPrivateBrowsing() to another thread
so we don't need to take the cache monitor on the main thread.

Review commit: https://reviewboard.mozilla.org/r/208992/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208992/

Comment hidden (mozreview-request)

It must be infallible for there is no way to propagate the error back to the
main thread when part of the init functions run on another thread. It is OK to
clone a stream that ends abnormally as long as we don't copy the error status
of EOS. The cloned stream will open a new channel when necessary.

Note we also copy the partial block from the original stream to get as much
data as possible and thus reducing the chance of reopening the channel.

Review commit: https://reviewboard.mozilla.org/r/208994/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208994/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/208996/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208996/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/208998/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208998/

JW Wang [:jwwang] [:jw_wang]

Assignee

Updated

•

6 years ago

Attachment #8938241 - Flags: review?(bechen)

Attachment #8938242 - Flags: review?(bechen)

Attachment #8938243 - Flags: review?(bechen)

Attachment #8938244 - Flags: review?(bechen)

Attachment #8938245 - Flags: review?(bechen)

Benjamin Chen [:bechen]

Comment 6

•

6 years ago

mozreview-review

Comment on attachment 8938241 [details]
Bug 1426578. P1 - tweak the constructor and init functions of ChannelMediaResource/MediaCacheStream.

https://reviewboard.mozilla.org/r/208990/#review214680

Attachment #8938241 - Flags: review?(bechen) → review+

Benjamin Chen [:bechen]

Comment 7

•

6 years ago

mozreview-review

Comment on attachment 8938242 [details]
Bug 1426578. P2 - always access MediaCache::mStreams while the cache monitor is held.

https://reviewboard.mozilla.org/r/208992/#review214688

Attachment #8938242 - Flags: review?(bechen) → review+

Benjamin Chen [:bechen]

Comment 8

•

6 years ago

mozreview-review

Comment on attachment 8938244 [details]
Bug 1426578. P4 - offload InitAsClone() to another thread.

https://reviewboard.mozilla.org/r/208996/#review214730

Attachment #8938244 - Flags: review?(bechen) → review+

Benjamin Chen [:bechen]

Comment 9

•

6 years ago

mozreview-review

Comment on attachment 8938243 [details]
Bug 1426578. P3 - make MediaCacheStream::InitAsClone() infallible.

https://reviewboard.mozilla.org/r/208994/#review214732

Attachment #8938243 - Flags: review?(bechen) → review+

Benjamin Chen [:bechen]

Comment 10

•

6 years ago

mozreview-review

Comment on attachment 8938245 [details]
Bug 1426578. P5 - offload Init() to another thread.

https://reviewboard.mozilla.org/r/208998/#review214734

Attachment #8938245 - Flags: review?(bechen) → review+

JW Wang [:jwwang] [:jw_wang]

Assignee

Updated

•

6 years ago

Attachment #8938241 - Flags: review?(gsquelart)

Attachment #8938242 - Flags: review?(gsquelart)

Attachment #8938243 - Flags: review?(gsquelart)

Attachment #8938244 - Flags: review?(gsquelart)

Attachment #8938245 - Flags: review?(gsquelart)

Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)

Comment 11

•

6 years ago

mozreview-review

Comment on attachment 8938241 [details]
Bug 1426578. P1 - tweak the constructor and init functions of ChannelMediaResource/MediaCacheStream.

https://reviewboard.mozilla.org/r/208990/#review214770

Attachment #8938241 - Flags: review?(gsquelart) → review+

Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)

Comment 12

•

6 years ago

mozreview-review

Comment on attachment 8938242 [details]
Bug 1426578. P2 - always access MediaCache::mStreams while the cache monitor is held.

https://reviewboard.mozilla.org/r/208992/#review214772

Attachment #8938242 - Flags: review?(gsquelart) → review+

Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)

Comment 13

•

6 years ago

mozreview-review

Comment on attachment 8938243 [details]
Bug 1426578. P3 - make MediaCacheStream::InitAsClone() infallible.

https://reviewboard.mozilla.org/r/208994/#review214774

Attachment #8938243 - Flags: review?(gsquelart) → review+

Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)

Comment 14

•

6 years ago

mozreview-review

Comment on attachment 8938244 [details]
Bug 1426578. P4 - offload InitAsClone() to another thread.

https://reviewboard.mozilla.org/r/208996/#review214776

Attachment #8938244 - Flags: review?(gsquelart) → review+

Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)

Comment 15

•

6 years ago

mozreview-review

Comment on attachment 8938245 [details]
Bug 1426578. P5 - offload Init() to another thread.

https://reviewboard.mozilla.org/r/208998/#review214778

Attachment #8938245 - Flags: review?(gsquelart) → review+

JW Wang [:jwwang] [:jw_wang]

Assignee

Comment 16

•

6 years ago

Thanks for the reviews!

Comment 17

•

6 years ago

Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a29b9105722b
P1 - tweak the constructor and init functions of ChannelMediaResource/MediaCacheStream. r=bechen,gerald
https://hg.mozilla.org/integration/autoland/rev/fb2edb2d8dd1
P2 - always access MediaCache::mStreams while the cache monitor is held. r=bechen,gerald
https://hg.mozilla.org/integration/autoland/rev/1f6caf89e382
P3 - make MediaCacheStream::InitAsClone() infallible. r=bechen,gerald
https://hg.mozilla.org/integration/autoland/rev/d005226af73b
P4 - offload InitAsClone() to another thread. r=bechen,gerald
https://hg.mozilla.org/integration/autoland/rev/46879b2506ca
P5 - offload Init() to another thread. r=bechen,gerald

Arthur Iakab [arthur_iakab]

Comment 18

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/a29b9105722b
https://hg.mozilla.org/mozilla-central/rev/fb2edb2d8dd1
https://hg.mozilla.org/mozilla-central/rev/1f6caf89e382
https://hg.mozilla.org/mozilla-central/rev/d005226af73b
https://hg.mozilla.org/mozilla-central/rev/46879b2506ca

Status: NEW → RESOLVED

Closed: 6 years ago

status-firefox59: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla59

You need to log in before you can comment on or make changes to this bug.