More synchronized ideas

[Jason House]

On Monday, 4 June 2012 at 17:40:38 UTC, Michel Fortin wrote:
> On 2012-06-04 13:15:57 +0000, "Jason House" 
> <jason.james.house at gmail.com> said:
>
>> If you really want to use synchronized classes, then you should
>> have two of them.
>
> Valid comment. I thought about creating yet another example 
> illustrating that, but I gave up when realizing the silliness 
> of it. I mean, yes you can make it work, but at the price of 
> writing a lot of boilerplate code just for forwarding things 
> around.
>
> Here's a modified implementation of that dictionary class, 
> wrapping the translations AA and the two counters in two 
> distinct classes:
>
> ...
>
> That said, the other point is that it's too easy to shoot 
> yourself in the foot using implicit synchronization. It's easy 
> to forget you still have a mutex locked when adding the call to 
> globalNotifyWordConfirmed, especially if you add this line of 
> code later in the development process and you have forgotten 
> about the "synchronized" keyword far away at the very top of 
> the class declaration. And once you have a deadlock and you've 
> identified the culprit, to fix the bug you need to split 
> everything that needs to be synchronized into a separate class, 
> a tiresome and bug-prone process, just because you can't 
> opt-out of the implicit synchronization. Call me nuts if you 
> want, but I think this is awful.


I was thinking something more like this:

shared class Dictionary
{
   private SynchronizedCounters counters;
   private SynchronizedStringMap translations(counters);

   void addWord(string word, string foreignWord) shared
   {
     // synchronized opIndexAssign call
     translations[word] = foreignWord;
   }
   bool confirmWord(string word, string foreignWord) shared
   {
     // synchronized opIndex call
     string candidate = translations[word];
     if (candidate != foreignWord)
       return false;
     counters.confirmOneWord();
     globalNotifyWordConfirmed(word, foreignWord);
     return true;
   }
}

// All counter operations are embedded here
// No need to review for any unsafe data usage,
// it's all here (future uses will add new methods)
synchronized class SynchronizedCounters
{
   private int confirmed, unconfirmed;
   void addUnconfirmed() { ++unconfirmed; }
   void confirmOneWord() { --unconfirmed; ++confirmed; }
}

// Similar concept, but embeds SynchronizedCounters
// I don't like that, but it's the only way to fully
// embrace D synchronized classes for toy example
synchronized class SynchronizedStringMap
{
   private string[string] translations;
   private SynchronizedCounters counters;
   this(SynchronizedCounters _counters)
     : counters(_counters) {}
   void opIndexAssign(string word, string foreignWord)
   {
     translations[word] = foreignWord;
     counters.addUnconfirmed();
   }
   string opIndex(string word)
   {
     shared string *found = word in translations;
     if (found)
       return *found;
     else
       return null;
   }
}




>
>
>> That being said, I've never used synchronized classes in my 
>> multithreaded D1/D2 code. I used Tango's Mutexes and 
>> Conditions. They're more flexible.
>
> I'd say it's a good choice. How does it work with shared 
> variables in D2, or are you just ignoring the type system?


I did not find my mutex based code, but did find a lock free 
broadcast queue.  It was written to be shared-aware.  Here's the 
receive class:
/// Receives delegates from the specified sender. Never blocks.
class receiver{
     private target parent;
     private shared sender source;
     private int nextMessageId = 1;
     this(target t, shared sender s){ parent = t; source = s; }
     bool receive(){
         if (source.receive(nextMessageId, parent)){
             nextMessageId++;
             return true;
         }
         return false;
     }
}

And here was the receive method in the sender class:
private bool receive(int messageId, target t) shared
{
     if (pending == 0 || id < messageId)
         return false;
     msg(t);
     atomicDecrement!(msync.raw)(pending);
     return true;
}

atomicDecrement in Tango was a template.  Looking at the commit 
logs, it looks like I had to hack at isValidNumericType, but I 
did not have to change the Atomic.d (beyond removing the volatile 
keywords and fixing incorrect assembly)