Hi Levente,  Hi Chris,

On Tue, Apr 28, 2015 at 3:41 PM, Levente Uzonyi <leves at elte.hu> wrote:

> There's no need to store preferences in a data structure at all. We
> already have "pragma" preferences (since 4.1), which store the preference
> values independently. Since the 4.1 release it's a "permanent" goal to
> rewrite all preferences to "pragma" preferences.
> We should just make it happen.


This seems like a lot of work, and is work that can be done over time.  But
right now we're suffering lock ups due to the Mutex in Preferences.  For
example, the Notifier/Debugger accesses the scrollBarsOnRight preference
and I've often seen lock ups caused by this.  So I propose that I fix the
access to be as I described it.  There be no access lock except for
adding/updating preferences.  So reading is done without synchronisation,
and setting and/or adding is done by copying and assigning.  I also propose
to compile preferences without creating a block, so

autoIndent
^ self
valueOfFlag: #autoIndent
ifAbsent: true

instead of

autoIndent
^ self
valueOfFlag: #autoIndent
ifAbsent: [true]

which is well-supported by both the Interpreter and the Cog VMs, given
Object>>value ^self.  This to save space and time.

Levente
>
> P.S.: Reverting that method will solve the concurrency issue.
>
>
> On Tue, 28 Apr 2015, Eliot Miranda wrote:
>
>
>>
>> On Tue, Apr 28, 2015 at 12:47 PM, Chris Muller <asqueaker at gmail.com>
>> wrote:
>>       Wait, the newer one has a non-local return in it, but
>>       Mutex>>#critical: has an ensure: in it anyway, so maybe I don't see
>>       the problem..?
>>
>>
>> If one hits ctrl-period when the system is in the critical section then
>> the debugger can't open because it interrupts the critical section,
>> preventing the ensure block from running, attempts to access e.g.
>> scroll bar preferences when it tries to open, and the system deadlocks.
>> So preferences either need to be *not* protected by a critical section, or
>> the Debugger needs not to access preferences.
>>
>> IMO, we should try and write preferences so that they don't require a
>> lock.  Writing them as a lock-free data structure would be a really good
>> idea. First that critical section is slow and clunky.  Second, I
>> presume it is there only for the rare case of a write to preferences, not
>> to protect reads.
>>
>> IMO, a simple implementation which copied and replaced the entire
>> preferences dictionary on write would be sufficient.  Sure there's a danger
>> that some client would get a stale value if it read concurrently
>> while there was a write, but then so what?  A preference is a preference,
>> not a hard-and-fast value, and code should work accessing a preference no
>> matter its value, so momentarily getting a stale value
>> shouldn't matter.  So the implementation could be as simple as
>>
>> addPreference: aName categories: categoryList default: aValue
>> balloonHelp: helpString projectLocal: localBoolean changeInformee:
>> informeeSymbol changeSelector: aChangeSelector type: aType
>> "Add or replace a preference as indicated.  Reuses the preexisting
>> Preference object for this symbol, if there is one, so that UI artifacts
>> that interact with it will remain valid."
>>
>> | aPreference aPrefSymbol |
>> aPrefSymbol := aName asSymbol.
>> aPreference := DictionaryOfPreferences
>> at: aPrefSymbol
>> ifAbsent:
>> [| newPreference |
>> newPreference := aPreference
>> name:aPrefSymbol
>> defaultValue:aValue
>> helpString:helpString
>> localToProject:localBoolean
>> categoryList:categoryList
>> changeInformee:informeeSymbol
>> changeSelector:aChangeSelector
>> type: aType.
>> AccessLock critical:
>> [| newDict |
>> newDict := DictionaryOfPreferences copy.
>> newDict at: aPrefSymbol put: newPreference].
>> self  compileAccessMethodForPreference:aPreference.
>> newPreference]
>>
>>
>>       On Tue, Apr 28, 2015 at 2:43 PM, Chris Muller <asqueaker at gmail.com>
>> wrote:
>>       >> The above change restores the old behavior of locking up the
>> image, so it
>>       >> should be reverted. An additional comment explaininng why aBlock
>> must not be
>>       >> evaluated inside the argument of
>> #accessDictionaryOfPreferencesIn: would be
>>       >> helpful.
>>       >
>>       > Ahh, because aBlock might have a non-local return in it, leaving
>> the
>>       > Mutex unsignaled (and critical unenterable), is that right?
>>       >
>>       > Took me a minute to see that problem.
>>       >
>>       > Okay, I'll revert that method if no one else does by my next
>> commit..
>>       >
>>       >> It would be even better to finally get rid of
>> DictionaryOfPreferences.
>>       >>
>>       >>
>>       >> Levente
>>       >>
>>
>>
>>
>>
>> --
>> best,Eliot
>>
>>
>
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20150630/e9beaefd/attachment-0001.htm