On 2012-11-11 18:46:10 +0000, Alex Rønne Petersen <alex@lycus.org> said:

> Something needs to be done about shared. I don't know what, but the current situation is -- and I'm really not exaggerating here -- laughable. I think we either need to just make it perfectly clear that shared is for documentation purposes and nothing else, or, figure out an alternative system to shared, because I don't see shared actually being useful for real world work no matter what we do with it.

I feel like the concurrency aspect of D2 was rushed in the haste of having it ready for TDPL. Shared, deadlock-prone synchronized classes[1] as well as destructors running in any thread (thanks GC!) plus a couple of other irritants makes the whole concurrency scheme completely flawed if you ask me. D2 needs a near complete overhaul on the concurrency front.

I'm currently working on a big code base in C++. While I do miss D when it comes to working with templates as well as for its compilation speed and a few other things, I can't say I miss D much when it comes to anything touching concurrency.

[1]: http://michelf.ca/blog/2012/mutex-synchonization-in-d/

-- 
Michel Fortin
michel.fortin@michelf.ca
http://michelf.ca/

On 11/11/2012 10:46 AM, Alex Rønne Petersen wrote:
> It's starting to get outright embarrassing to talk to newcomers about D's
> concurrency support because the most fundamental part of it -- the shared type
> qualifier -- does not have well-defined semantics at all.

I think a couple things are clear:

1. Slapping shared on a type is never going to make algorithms on that type work in a concurrent context, regardless of what is done with memory barriers. Memory barriers ensure sequential consistency, they do nothing for race conditions that are sequentially consistent. Remember, single core CPUs are all sequentially consistent, and still have major concurrency problems. This also means that having templates accept shared(T) as arguments and have them magically generate correct concurrent code is a pipe dream.

2. The idea of shared adding memory barriers for access is not going to ever work. Adding barriers has to be done by someone who knows what they're doing for that particular use case, and the compiler inserting them is not going to substitute.


However, and this is a big however, having shared as compiler-enforced self-documentation is immensely useful. It flags where and when data is being shared. So, your algorithm won't compile when you pass it a shared type? That is because it is NEVER GOING TO WORK with a shared type. At least you get a compile time indication of this, rather than random runtime corruption.

To make a shared type work in an algorithm, you have to:

1. ensure single threaded access by aquiring a mutex
2. cast away shared
3. operate on the data
4. cast back to shared
5. release the mutex

Also, all op= need to be disabled for shared types.

The only problem beeing that you can not really have user defined shared (value) types:

http://d.puremagic.com/issues/show_bug.cgi?id=8295

Kind Regards
Benjamin Thaut

On 11/11/2012 10:05 PM, Benjamin Thaut wrote:
> The only problem beeing that you can not really have user defined shared (value)
> types:
>
> http://d.puremagic.com/issues/show_bug.cgi?id=8295

If you include an object designed to work only in a single thread (non-shared), make it shared, and then destruct it when other threads may be pointing to it ...

What should happen?

Am Sun, 11 Nov 2012 18:30:17 -0800
schrieb Walter Bright <newshound2@digitalmars.com>:

> 
> To make a shared type work in an algorithm, you have to:
> 
> 1. ensure single threaded access by aquiring a mutex
> 2. cast away shared
> 3. operate on the data
> 4. cast back to shared
> 5. release the mutex
> 
> Also, all op= need to be disabled for shared types.

But there are also shared member functions and they're kind of annoying right now:

* You can't call shared methods from non-shared methods or vice versa.
  This leads to code duplication, you basically have to implement
  everything twice:

----------
struct ABC
{
        Mutext mutex;
	void a()
	{
		aImpl();
	}
	shared void a()
	{
		synchronized(mutex)
		    aImpl();  //not allowed
	}
	private void aImpl()
	{

	}
}
----------
The only way to avoid this is casting away shared in the shared a method, but that really is annoying.

* You can't have data members be included only for the shared version.
  In the above example, the mutex member will always be included, even
  if ABC instance is thread local.

So you're often better off writing a non-thread safe struct and writing
a wrapper struct. This way you don't have useless overhead in the
non-thread safe implementation. But the nice instance syntax is
lost:

shared(ABC) abc1; ABC abc2;
vs
SharedABC abc1; ABC abc2;

even worse, shared propagation won't work this way;

struct DEF
{
    ABC abc;
}
shared(DEF) def;
def.abc.a();



and then there's also the druntime issue: core.sync doesn't work with
shared which leads to this schizophrenic situation:
struct A
{
    Mutex m;
    void a() //Doesn't compile with shared
    {
        m.lock();  //Compiles, but locks on a TLS mutex!
        m.unlock();
    }
}

struct A
{
    shared Mutex m;
    shared void a()
    {
        m.lock();  //Doesn't compile
        (cast(Mutex)m).unlock(); //Ugly
    }
}

So the only useful solution avoids using shared:
struct A
{
    __gshared Mutex m; //Good we have __gshared!
    shared void a()
    {
        m.lock();
        m.unlock();
    }
}



And then there are some open questions with advanced use cases:
* How do I make sure that a non-shared delegate is only accepted if I
  have an A, but a shared delegate should be supported
  for shared(A) and A? (calling a shared delegate from a non-shared
  function should work, right?)

struct A
{
    void a(T)(T v)
    {
        writeln("non-shared");
    }
    shared void a(T)(T v)  if (isShared!v) //isShared doesn't exist
    {
        writeln("shared");
    }
}

And having fun with this little example: http://dpaste.dzfl.pl/7f6a4ad2

* What's the difference between: "void delegate() shared"
  and "shared(void delegate())"?

Error: cannot implicitly convert expression (&a.abc) of type void
delegate() shared to shared(void delegate())

* So let's call it void delegate() shared instead:
void incrementA(void delegate() shared del)
/home/c684/c922.d(7): Error: const/immutable/shared/inout attributes
  are only valid for non-static member functions

On 11/12/2012 2:57 AM, Johannes Pfau wrote:
> But there are also shared member functions and they're kind of annoying
> right now:
>
> * You can't call shared methods from non-shared methods or vice versa.
>    This leads to code duplication, you basically have to implement
>    everything twice:

You can't get away from the fact that data that can be accessed from multiple threads has to be dealt with in a *fundamentally* different way than single threaded code. You cannot share code between the two. There is simply no conceivable way that "share" can be added and then code will become thread safe.

Most of the issues you're having seem to revolve around treating shared data access just like single threaded access, except "share" was added. This cannot work. The compiler error messages, while very annoying, are in their own obscure way pointing this out.

It's my fault, I have not explained share very well, and have oversold it. It does not solve concurrency problems, it points them out.

>
> ----------
> struct ABC
> {
>          Mutext mutex;
> 	void a()
> 	{
> 		aImpl();
> 	}
> 	shared void a()
> 	{
> 		synchronized(mutex)
> 		    aImpl();  //not allowed
> 	}
> 	private void aImpl()
> 	{
> 		
> 	}
> }
> ----------
> The only way to avoid this is casting away shared in the shared a
> method, but that really is annoying.

As I explained, the way to manipulate shared data is to get exclusive access to it via a mutex, cast away the shared-ness, manipulate it as single threaded data, convert it back to shared, and release the mutex.


>
> * You can't have data members be included only for the shared version.
>    In the above example, the mutex member will always be included, even
>    if ABC instance is thread local.
>
> So you're often better off writing a non-thread safe struct and writing
> a wrapper struct. This way you don't have useless overhead in the
> non-thread safe implementation. But the nice instance syntax is
> lost:
>
> shared(ABC) abc1; ABC abc2;
> vs
> SharedABC abc1; ABC abc2;
>
> even worse, shared propagation won't work this way;
>
> struct DEF
> {
>      ABC abc;
> }
> shared(DEF) def;
> def.abc.a();
>
>
>
> and then there's also the druntime issue: core.sync doesn't work with
> shared which leads to this schizophrenic situation:
> struct A
> {
>      Mutex m;
>      void a() //Doesn't compile with shared
>      {
>          m.lock();  //Compiles, but locks on a TLS mutex!
>          m.unlock();
>      }
> }
>
> struct A
> {
>      shared Mutex m;
>      shared void a()
>      {
>          m.lock();  //Doesn't compile
>          (cast(Mutex)m).unlock(); //Ugly
>      }
> }
>
> So the only useful solution avoids using shared:
> struct A
> {
>      __gshared Mutex m; //Good we have __gshared!
>      shared void a()
>      {
>          m.lock();
>          m.unlock();
>      }
> }

Yes, mutexes will need to exist in a global space.

>
>
> And then there are some open questions with advanced use cases:
> * How do I make sure that a non-shared delegate is only accepted if I
>    have an A, but a shared delegate should be supported
>    for shared(A) and A? (calling a shared delegate from a non-shared
>    function should work, right?)
>
> struct A
> {
>      void a(T)(T v)
>      {
>          writeln("non-shared");
>      }
>      shared void a(T)(T v)  if (isShared!v) //isShared doesn't exist
>      {
>          writeln("shared");
>      }
> }

First, you have to decide what you mean by a shared delegate. Do you mean the variable containing the two pointers that make up a delegate are shared, or the delegate is supposed to deal with shared data?


>
> And having fun with this little example:
> http://dpaste.dzfl.pl/7f6a4ad2
>
> * What's the difference between: "void delegate() shared"
>    and "shared(void delegate())"?
>
> Error: cannot implicitly convert expression (&a.abc) of type void
> delegate() shared

The delegate deals with shared data.

> to shared(void delegate())

The variable holding the delegate is shared.


> * So let's call it void delegate() shared instead:
> void incrementA(void delegate() shared del)
> /home/c684/c922.d(7): Error: const/immutable/shared/inout attributes
>    are only valid for non-static member functions

Am 11.11.2012 19:46, schrieb Alex Rønne Petersen:
> Something needs to be done about shared. I don't know what, but the current situation is -- and I'm really not exaggerating here -- laughable. I think we either need to just make it perfectly clear that shared is for documentation purposes and nothing else, or, figure out an alternative system to shared, because I don't see shared actually being useful for real world work no matter what we do with it.
> 

After reading Walter's comment, it suddenly seemed obvious that we are currently using 'shared' the wrong way. Shared is just not meant to be used on objects at all (or only in some special cases like synchronization primitives). I just experimented a bit with a statically checked library based solution and a nice way to use shared is to only use it for disabling access to non-shared members while its monitor is not locked. A ScopedLock proxy and a lock() function can be used for this:

---
class MyClass {
	void method();
}

void main()
{
	auto inst = new shared(MyClass);
	//inst.method(); // forbidden

	{
		ScopedLock!MyClass l = lock(inst);
		l.method(); // now allowed as long as 'l' is in scope
	}

	// can also be called like this:
	inst.lock().method();
}
---

ScopedLock is non-copyable and handles the dirty details of locking and casting away 'shared' when its safe to do so. No tagging of the class with 'synchronized' or 'shared' needs to be done and everything works nicely without casts.

This comes with a restriction, though. Doing all this is only safe as long as the instance is known to not contain any unisolated aliasing*. So use would be restricted to types that contain only immutable or unique/isolated references.

So I also implemented an Isolated!(T) type that is recognized by
ScopedLock, as well as functions such as spawn(). The resulting usage
can be seen in the example at the bottom.

It doesn't provide all the flexibility that a built-in 'isolated' type would do, but the possible use cases at least look interesting. There are still some details to be worked out, such as writing a spawn() function that correctly moves Isolated!() parameters instead of copying or the forward reference error mentioned in the example.

I'll now try and see if some of my earlier multi-threading designs fit into this system.

---
import std.stdio;
import std.typecons;
import std.traits;
import stdx.typecons;

class SomeClass {

}

class Test {
	private {
		string m_test1 = "test 1";
		Isolated!SomeClass m_isolatedReference;
		// currently causes a size forward reference error:
		//Isolated!Test m_next;
	}

	this()
	{
		//m_next = ...;
	}

	void test1() const { writefln(m_test1); }
	void test2() const { writefln("test 2"); }
}

void main()
{
	writefln("Shared locking");
	// create a shared instance of Test - no members will
	// be accessible
	auto t = new shared(Test);
	{
		// temporarily lock t to make all non-shared members
		// safely available
		// lock() words only for objects with no unisolated
		// aliasing.
		ScopedLock!Test l = lock(t);
		l.test1();
		l.test2();
	}

	// passing a shared object to a different thread works as usual
	writefln("Shared spawn");
	spawn(&myThreadFunc1, t);

	// create an isolated instance of Test
	// currently, Test may not contain unisolated aliasing, but
	// this requirement may get lifted,
	// as long as only pure methods are called
	Isolated!Test u = makeIsolated!Test();

	// move ownership to a different function and recover
	writefln("Moving unique");
	Isolated!Test v = myThreadFunc2(u.move());

	// moving to a different thread also works
	writefln("Moving unique spawn");
	spawn(&myThreadFunc2, v.move());

	// another possibility is to convert to immutable
	auto w = makeIsolated!Test();
	writefln("Convert to immutable spawn");
	spawn(&myThreadFunc3, w.freeze());

	// or just loose the isolation and act on the base type
	writefln("Convert to mutable");
	auto x = makeIsolated!Test();
	Test xm = x.extract();
	xm.test1();
	xm.test2();
}

void myThreadFunc1(shared(Test) t)
{
	// call non-shared method on shared object
	t.lock().test1();
	t.lock().test2();
}

Isolated!Test myThreadFunc2(Isolated!Test t)
{
	// call methods as usual on an isolated object
	t.test1();
	t.test2();
	return t.move();
}

void myThreadFunc3(immutable(Test) t)
{
	t.test1();
	t.test2();
}


// fake spawn function just to test the type constraints
void spawn(R, ARGS...)(R function(ARGS) func, ARGS args)
{
	foreach( i, T; ARGS )
		static assert(!hasUnisolatedAliasing!T ||
			!hasUnsharedAliasing!T,
			"Parameter "~to!string(i)~" of type"
			~T.stringof~" has unshared or unisolated
			 aliasing. Cannot safely be passed to a
			different thread.");

	// TODO: do this in a different thread...
	// TODO: don't cheat with the 1-parameter move detection
	static if(__traits(compiles, func(args[0])) ) func(args);
	else func(args[0].move());
}
---


* shared aliasing would also be OK, but this is not yet handled by the implementation.

On Mon, 12 Nov 2012 02:30:17 -0000, Walter Bright <newshound2@digitalmars.com> wrote:
> To make a shared type work in an algorithm, you have to:
>
> 1. ensure single threaded access by aquiring a mutex
> 2. cast away shared
> 3. operate on the data
> 4. cast back to shared
> 5. release the mutex

So what we actually want, in order to make the above "nice" is a "scoped" struct wrapping the mutex and shared object which does all the "dirty" work for you.  I'm thinking..

// (0)
with(ScopedLock(obj,lock))  // (1)
{
  obj.foo = 2;              // (2)
}                           // (3)
// (4)

(0) obj is a "shared" reference, lock is a global mutex
(1) mutex is acquired here, shared is cast away
(2) 'obj' is not "shared" here so data access is allowed
(3) ScopedLock is "destroyed" and the mutex released
(4) obj is shared again

I think most of the above can be done without any compiler support but it would be "nice" if the compiler did something clever with 'obj' such that it knew it wasn't 'shared' inside the the 'with' above.  If not, if a full library solution is desired we could always have another temporary "unshared" variable referencing obj.

R

-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/

On Mon, 12 Nov 2012 11:55:51 -0000, Regan Heath <regan@netmail.co.nz> wrote:
> On Mon, 12 Nov 2012 02:30:17 -0000, Walter Bright <newshound2@digitalmars.com> wrote:
>> To make a shared type work in an algorithm, you have to:
>>
>> 1. ensure single threaded access by aquiring a mutex
>> 2. cast away shared
>> 3. operate on the data
>> 4. cast back to shared
>> 5. release the mutex
>
> So what we actually want, in order to make the above "nice" is a "scoped" struct wrapping the mutex and shared object which does all the "dirty" work for you.  I'm thinking..
>
> // (0)
> with(ScopedLock(obj,lock))  // (1)
> {
>    obj.foo = 2;              // (2)
> }                           // (3)
> // (4)
>
> (0) obj is a "shared" reference, lock is a global mutex
> (1) mutex is acquired here, shared is cast away
> (2) 'obj' is not "shared" here so data access is allowed
> (3) ScopedLock is "destroyed" and the mutex released
> (4) obj is shared again
>
> I think most of the above can be done without any compiler support but it would be "nice" if the compiler did something clever with 'obj' such that it knew it wasn't 'shared' inside the the 'with' above.  If not, if a full library solution is desired we could always have another temporary "unshared" variable referencing obj.

There was talk a while back about how to handle the existing object mutex and synchronized{} statement blocks and this subject has me thinking back to that.  My thinking has gone full circle and rather than bore you with all the details I want to present a conclusion which I am hoping is both implementable and useful.

First off, IIRC object contains a mutex/monitor/critical section, which means all objects contain one.  The last discussion saw many people wanting this removed for efficiency.  I propose we do this.  Then, if a class or struct is declared as "shared" or a "shared" instance of a class or struct is constructed we magically include one (compiler magic which I hope is possible).

Secondly I say we make "shared" illegal on basic types.  This is a limitation(*) but I believe in most cases a single int is unlikely to be shared without an accompanying group of other variables, and usually an algorithm operating on those variables.  These variables and the algorithm should be encapsulated in a class or struct - which can in turn be shared.

Now.. the synchronized() {} statement can do the magic described above (as ScopedLock) for us.  It would be illegal to call it on a non "shared" instance.  It would acquire the mutex and cast away "shared" inside the block/scope, at the end of the scope it would cast shared back and release the mutex.

(*) for those rare cases where a single int or other basic type is all that is shared we can provide a wrapper struct which is declared as "shared".

R

-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/

On Mon, 12 Nov 2012 11:41:00 -0000, Sönke Ludwig <sludwig@outerproduct.org> wrote:

> Am 11.11.2012 19:46, schrieb Alex Rønne Petersen:
>> Something needs to be done about shared. I don't know what, but the
>> current situation is -- and I'm really not exaggerating here --
>> laughable. I think we either need to just make it perfectly clear that
>> shared is for documentation purposes and nothing else, or, figure out an
>> alternative system to shared, because I don't see shared actually being
>> useful for real world work no matter what we do with it.
>>
>
> After reading Walter's comment, it suddenly seemed obvious that we are
> currently using 'shared' the wrong way. Shared is just not meant to be
> used on objects at all (or only in some special cases like
> synchronization primitives). I just experimented a bit with a statically
> checked library based solution and a nice way to use shared is to only
> use it for disabling access to non-shared members while its monitor is
> not locked. A ScopedLock proxy and a lock() function can be used for this:

I had exactly the same idea:
http://forum.dlang.org/thread/k7orpj$1tt5$1@digitalmars.com?page=2#post-op.wnnsrds954xghj:40puck.auriga.bhead.co.uk

But, then I went right back the other way:
http://forum.dlang.org/thread/k7orpj$1tt5$1@digitalmars.com?page=2#post-op.wnnt4iyz54xghj:40puck.auriga.bhead.co.uk

I think we can definitely create a library solution like the one you propose below, and it should work quite well.  But, I reckon it would be even nicer if the compiler did just a little bit of the work for us, and we integrated with the built in synchronized statement. :)

R

-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/