Closed
Bug 989020
Opened 10 years ago
Closed 10 years ago
Write a helper to make it easy to access preferences off-Main Thread
Categories
(Core :: Preferences: Backend, defect)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: mikeh, Assigned: mikeh)
References
Details
Attachments
(1 file, 1 obsolete file)
16.75 KB,
patch
|
benjamin
:
review-
bent.mozilla
:
feedback-
|
Details | Diff | Splinter Review |
No description provided.
Comment 1•10 years ago
|
||
Could you be more specific? In general I think it should rarely if ever be necessary to access real preferences from other threads.
Flags: needinfo?(mhabicher)
Assignee | ||
Comment 2•10 years ago
|
||
How does this look for a first pass?
Attachment #8398190 -
Flags: feedback?(fabrice)
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #1) > > Could you be more specific? In general I think it should rarely if ever be > necessary to access real preferences from other threads. For myself, I've been working on shim layers to improve test coverage of b2g's camera code, and right now the mochitests push test configurations through prefs, which get picked up on a worker thread in Gecko. Bug bug 980419 in mind, I found myself bouncing nsRunnables off of the Main Thread to get prefs, and figured it would be easier if there were something to handle that for me, like the attached code.
Flags: needinfo?(mhabicher)
Assignee | ||
Comment 4•10 years ago
|
||
Attach the complete patch, this time.
Attachment #8398190 -
Attachment is obsolete: true
Attachment #8398190 -
Flags: feedback?(fabrice)
Attachment #8398192 -
Flags: feedback?(fabrice)
Comment 5•10 years ago
|
||
This will help me fix bug 980027 as well. Thanks Mike!
Comment 6•10 years ago
|
||
Comment on attachment 8398192 [details] [diff] [review] WIP - OffMainThreadPreferences, v1 Review of attachment 8398192 [details] [diff] [review]: ----------------------------------------------------------------- Mike, I have no time to look at that today and will be PTO next week. Better find someone else...
Attachment #8398192 -
Flags: feedback?(fabrice)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8398192 [details] [diff] [review] WIP - OffMainThreadPreferences, v1 bent, what do you think?
Attachment #8398192 -
Flags: feedback?(bent.mozilla)
Comment 8•10 years ago
|
||
Comment on attachment 8398192 [details] [diff] [review] WIP - OffMainThreadPreferences, v1 This is not a good pattern. To the extent that we actually need to control non-mainthread code using prefs, we should record the pref values either at startup or when we launch the task in question.
Attachment #8398192 -
Flags: review-
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #8) > > This is not a good pattern. > > To the extent that we actually need to control non-mainthread code using > prefs, we should record the pref values either at startup or when we launch > the task in question. What should be done in the case where this is not possible? Currently people either pull prefs off-Main Thread (see bug bug 980419) or they will code up their own repetitive solutions to bouncing events off of the Main Thread.
Flags: needinfo?(benjamin)
Comment 10•10 years ago
|
||
I haven't seen any of those cases. I don't know what the threading model of GonkCameraManager or ICamera is (no docs or comments!?) but it seems that the main-thread factories for those objects should collect the necessary prefs, or in the worst case the content module constructor should archive the necessary values in atomic globals. In no case does it make sense to bounce events around.
Flags: needinfo?(benjamin)
Assignee | ||
Comment 11•10 years ago
|
||
The issue is the necessary prefs can't be collected all at once because they change over time as the automated tests are running.
Comment on attachment 8398192 [details] [diff] [review] WIP - OffMainThreadPreferences, v1 In general there are two patterns that I've needed in workers: 1. Grab prefs at startup (or the first time a worker is created) and stash them in a readonly spot (const global, etc). 2. Install pref observers that monitor prefs that I expect to change and will need to respond to. These observers send messages to running workers so that they update themselves. In general trying to do things magically will almost always have unintended consequences I think. Your patch uses NS_DISPATCH_SYNC for instance, and that will spin the event loop on the calling thread (assuming the calling thread is an nsThread and not something simpler like a chromium/NSPR/native thread... in which case I guess it'll just fail).
Attachment #8398192 -
Flags: feedback?(bent.mozilla) → feedback-
Assignee | ||
Comment 13•10 years ago
|
||
smedberg, bent--thanks for the feedback. I'm going to close this bug and open a new one with a Camera-specific based on comment 12.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•