Make ImageContainer::AllocateProducerID callable on all threads

RESOLVED FIXED in Firefox 44

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kikuo, Unassigned)

Tracking

unspecified
mozilla44
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
According to Bug 1194918 Comment 39, 40, 43, to make AllocateProducerID() callable on all threads can make it used more widely.
(Reporter)

Comment 1

2 years ago
Not clear which component the bug should belong to ...
(Reporter)

Comment 2

2 years ago
Created attachment 8671155 [details]
MozReview Request: Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r=roc.

Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r?roc
Attachment #8671155 - Flags: review?(roc)
Comment on attachment 8671155 [details]
MozReview Request: Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r=roc.

https://reviewboard.mozilla.org/r/21547/#review19369
Attachment #8671155 - Flags: review?(roc) → review+
(Reporter)

Comment 4

2 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> Comment on attachment 8671155 [details]
> MozReview Request: Bug 1212288 - Make ImageContainer::AllocateProducerID
> callable on all threads; r?roc
> 
> https://reviewboard.mozilla.org/r/21547/#review19369

Thanks for review.
https://reviewboard.mozilla.org/r/21547/#review19371

::: gfx/layers/ImageContainer.cpp:676
(Diff revision 1)
> -static ImageContainer::ProducerID sProducerID = 0;
> +static Atomic<ImageContainer::ProducerID> sProducerID(0u);

Is the constructor of Atomic trivial enough for the compiler not to emit a static initializer?

Not sure if this guideline is still relevant?
https://developer.mozilla.org/en-US/docs/Mozilla/C%2B%2B_Portability_Guide#Don't_use_static_constructors
(Reporter)

Comment 6

2 years ago
I'm not sure if the compiler bug mentioned in guideline still happens nowadays.
I can apply Construct on first use idiom here.
(Reporter)

Comment 7

2 years ago
Comment on attachment 8671155 [details]
MozReview Request: Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r=roc.

Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r=roc.
Construct the static variable on first use to avoid this potential issue mentioned here https://developer.mozilla.org/en-US/docs/Mozilla/C%2B%2B_Portability_Guide#Don%27t_use_static_constructors
Attachment #8671155 - Attachment description: MozReview Request: Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r?roc → MozReview Request: Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r=roc.
Attachment #8671155 - Flags: review+ → review?(roc)
Comment on attachment 8671155 [details]
MozReview Request: Bug 1212288 - Make ImageContainer::AllocateProducerID callable on all threads; r=roc.

https://reviewboard.mozilla.org/r/21547/#review19373
(Reporter)

Comment 9

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=895689c4e5ea
Don't know why the task queue for windows platform is getting so long ... but the results on all other platforms look just fine.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db193b0893a1
(Reporter)

Updated

2 years ago
Keywords: checkin-needed
(Reporter)

Updated

2 years ago
Blocks: 1194918
https://hg.mozilla.org/mozilla-central/rev/1d0b33ab526a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.