Re: PSA: Major preference service architecture changes inbound

On 20/07/2018 03:25, Kris Maglione wrote: On Thu, Jul 19, 2018 at 07:17:13PM -0700, Jeff Gilbert wrote: Using a classic read/write exclusive lock, we would only every contend on read+write or write+write, which are /rare/. That might be true if we gave up on the idea of switching to Robin

Re: PSA: Major preference service architecture changes inbound

I have filed bug 1477436: "Preferences::Get* is not threadsafe/is main thread only" https://bugzilla.mozilla.org/show_bug.cgi?id=1477436 On Fri, Jul 20, 2018 at 11:36 AM, Eric Rahm wrote: > We *could* special case prefs with an appropriate data structure that works > in a thread-safe nature; as

Re: PSA: Major preference service architecture changes inbound

Since I have seen several people point out in this thread that there's *probably* code that excessively accesses prefs: You can easily assert the name and amount of different prefs that are read during whatever scenario you'd like to perform by adding to this test (or writing your own version of

Re: PSA: Major preference service architecture changes inbound

On Fri, Jul 20, 2018 at 11:53:22AM -0400, Ehsan Akhgari wrote: On Fri, Jul 20, 2018 at 5:00 AM, Jonathan Kew wrote: +1 to that. Our need for OMT access to prefs is only likely to grow, IMO, and we should just fix it once, so that any code (regardless of which thread(s) it may eventually run

Re: PSA: Major preference service architecture changes inbound

On Fri, Jul 20, 2018 at 10:00:22AM +0100, Jonathan Kew wrote: +1 to that. Our need for OMT access to prefs is only likely to grow, IMO, and we should just fix it once, so that any code (regardless of which thread(s) it may eventually run on) can trivially read prefs. Even if that means we

Re: PSA: Major preference service architecture changes inbound

We *could* special case prefs with an appropriate data structure that works in a thread-safe nature; as far as RWLock's go, we do have one in tree [1]. This has gone off the rails a bit from Kris' original announcement, which I'll reiterate: Watch out for prefs related bustage. Jeff, would you

Re: PSA: Major preference service architecture changes inbound

Hi I believe that this change may be the caused of https://bugzilla.mozilla.org/show_bug.cgi?id=1477254 That is, the pref value set in all.js no longer overrides the default value set in StaticPrefs. The problem occurs mostly with e10s

Re: PSA: Major preference service architecture changes inbound

On Fri, Jul 20, 2018 at 5:00 AM, Jonathan Kew wrote: > On 20/07/2018 03:17, Jeff Gilbert wrote: > >> Using a classic read/write exclusive lock, we would only every contend >> on read+write or write+write, which are /rare/. >> > > Indeed, that's what I envisage we'd want. The -vast- majority of

Re: PSA: Major preference service architecture changes inbound

On 20/07/2018 03:17, Jeff Gilbert wrote: Using a classic read/write exclusive lock, we would only every contend on read+write or write+write, which are /rare/. Indeed, that's what I envisage we'd want. The -vast- majority of prefs access only involves reading values. We should be able to do

Re: PSA: Major preference service architecture changes inbound

On Fri, Jul 20, 2018 at 7:37 AM, Daniel Veditz wrote: > ​Prefs might be a terrible way to implement that functionality, but it's > been used that way as long as we've had prefs in Mozilla so there seems to > be a need for it. Early offenders: printer setup, mail accounts, external > protocol

Re: PSA: Major preference service architecture changes inbound

Using a classic read/write exclusive lock, we would only every contend on read+write or write+write, which are /rare/. It's really, really nice when we can have dead-simple threadsafe APIs, instead of requiring people to jump through hoops or roll their own dispatch code. (fragile) IIRC most new

Re: PSA: Major preference service architecture changes inbound

On Thu, Jul 19, 2018 at 02:37:07PM -0700, Justin Dolske wrote: I know we've had code that, instead of reading a pref directly, checks the pref once in an init() and uses pref observers to watch for any changes to it. (i.e., basically mirrors the pref into some module-local variable, at which

Re: PSA: Major preference service architecture changes inbound

On Tue, Jul 17, 2018 at 9:23 PM, Nicholas Nethercote wrote: > This is a good example of how prefs is a far more general mechanism than I > would like, leading to all manner of use and abuse. "All I want is a > key-value store, with fast multi-threaded access, where the keys aren't > known ahead

Re: PSA: Major preference service architecture changes inbound

I know we've had code that, instead of reading a pref directly, checks the pref once in an init() and uses pref observers to watch for any changes to it. (i.e., basically mirrors the pref into some module-local variable, at which point you can roll your own locking or whatever to make it

Re: PSA: Major preference service architecture changes inbound

On Tue, Jul 17, 2018 at 03:49:41PM -0700, Jeff Gilbert wrote: We should totally be able to afford the very low cost of a rarely-contended lock. What's going on that causes uncached pref reads to show up so hot in profiles? Do we have a list of problematic pref keys? So, at the moment, we read

Re: PSA: Major preference service architecture changes inbound

Nicholas Nethercote wrote on 2018-07-17 21:23: This is a good example of how prefs is a far more general mechanism than I would like, leading to all manner of use and abuse. "All I want is a key-value store, with fast multi-threaded access, where the keys aren't known ahead of time." Agreed, the

Re: PSA: Major preference service architecture changes inbound

On Wed, Jul 18, 2018 at 1:57 AM, Kris Maglione wrote: > > While we're thinking about the prefs service, is there any possibility we >> could enable off-main-thread access to preferences? >> > > I think the chances of that are pretty close to 0, but I'll defer to Nick. > I agree, for the reasons

Re: PSA: Major preference service architecture changes inbound

We should totally be able to afford the very low cost of a rarely-contended lock. What's going on that causes uncached pref reads to show up so hot in profiles? Do we have a list of problematic pref keys? On Tue, Jul 17, 2018 at 8:57 AM, Kris Maglione wrote: > On Tue, Jul 17, 2018 at 02:06:48PM

Re: PSA: Major preference service architecture changes inbound

On Tue, Jul 17, 2018 at 02:06:48PM +0100, Jonathan Kew wrote: On 13/07/2018 21:37, Kris Maglione wrote: tl;dr: A major change to the architecture preference service has just landed, so please be on the lookout for regressions. We've been working for the last few weeks on rearchitecting the

Re: PSA: Major preference service architecture changes inbound

On 13/07/2018 21:37, Kris Maglione wrote: tl;dr: A major change to the architecture preference service has just landed, so please be on the lookout for regressions. We've been working for the last few weeks on rearchitecting the preference service to work better in our current and future