On Monday, 4 June 2012 at 17:40:38 UTC, Michel Fortin wrote:
On 2012-06-04 13:15:57 +0000, "Jason House"
<[email protected]> 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)
