www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - implicit conversions to/from shared

reply ag0aep6g <anonymous example.com> writes:
While messing with atomicLoad [1], I noticed that dmd lets me implicitly 
convert values to/from shared without restrictions. It's in the spec 
[2]. This seems bad to me.

Here's how I understand shared. If anything's wrong, I'd appreciate if 
someone would educate me.

Shared is there to prevent me from accidentally writing code that's not 
thread-safe (at a low level).

When I never use shared (or __gshared or other mean hacks), I only ever 
get thread-local variables and I can't share data between threads. I get 
thread safety by avoiding the issue entirely.

When I do use shared, the compiler is supposed to reject (low-level) 
operations that are not thread-safe. That's why ++x is deprecated for a 
shared x, and I'm told to use atomicOp!"+=" instead.

Of course I can still do a verbose unsafe version which the compiler 
can't catch:

     atomicStore(x, atomicLoad(x) + 1);

The compiler can't possibly know which high-level operations need to run 
uninterrupted, so there's nothing it can do about this.

Back to implicit conversions. Loading and storing is as low-level as it 
gets. When D disallows unsafe ++ on shared, surely it should also 
disallow unsafe loading and storing.

Copying a shared value type to an unshared one could be defined as doing 
an atomic load, and copying the other way around could result in an 
atomic store. I don't think it's specified or implemented like this at 
the moment, but it would make sense to me. Loading/storing overly large 
value types can probably not be made thread-safe like that.

So, conclusion/proposal:

* Conversion of a value type from shared to unshared should only be 
allowed if it can be done with an atomic load. The compiler should 
generate the atomic load.
* Conversion of a value type from unshared to shared should only be 
allowed if it can be done with an atomic store. The compiler should 
generate the atomic store.
* Other conversions are not allowed.

Or alternatively, simply disallow all those conversions, and tell the 
user that they have to ensure thread safety themselves, e.g. by using 
atomicLoad/atomicStore.

Sorry for the noise if this is all known and part of the "shared isn't 
implemented" mantra. I couldn't find a bugzilla issue.


[1] https://github.com/dlang/druntime/pull/1605
[2] https://dlang.org/spec/const3.html#implicit_conversions (first sentence)
Jul 10 2016
next sibling parent reply Alex Parrill <initrd.gz gmail.com> writes:
On Sunday, 10 July 2016 at 13:02:17 UTC, ag0aep6g wrote:
 While messing with atomicLoad [1], I noticed that dmd lets me 
 implicitly convert values to/from shared without restrictions. 
 It's in the spec [2]. This seems bad to me.

 [...]
Atomic loading and storing, from what I understand, is usually limited to about a word on most architectures. I don't think it would be good to implicitly define assignment and referencing as atomic operations, since it would be limited. IMO concurrent access is better off being explicit anyway. I don't think there is an issue with converting unshared reference types to shared (ex. ref T -> ref shared(T) or T* -> shared(T)*); worst you get is some extra synchronization.
Jul 10 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/11/2016 01:52 AM, Alex Parrill wrote:
 Atomic loading and storing, from what I understand, is usually limited
 to about a word on most architectures. I don't think it would be good to
 implicitly define assignment and referencing as atomic operations, since
 it would be limited. IMO concurrent access is better off being explicit
 anyway.
Simply disallow reading and writing shared then, forcing something more explicit like atomicLoad/atomicStore? That would be better than the current state, but it would make shared even more unwieldy. Generating atomic operations would break less code, and feels like the obvious thing to me. It would be kinda explicit: shared being involved gives it away. Also, with other forms of ensuring thread safety, you have to cast shared away anyway, so atomic reading/writing wouldn't be applied there.
 I don't think there is an issue with converting unshared reference types
 to shared (ex. ref T -> ref shared(T) or T* -> shared(T)*); worst you
 get is some extra synchronization.
shared(T)* is not shared itself, so that's fine, yeah. I think I've approached this from the wrong angle. Implicit conversions are not the problem. Reading/writing is. Copying a shared value to another shared location is unsafe, too, if not done atomically.
Jul 10 2016
next sibling parent ag0aep6g <anonymous example.com> writes:
On 07/11/2016 07:26 AM, ag0aep6g wrote:
 On 07/11/2016 01:52 AM, Alex Parrill wrote:
[...]
 I don't think there is an issue with converting unshared reference types
 to shared (ex. ref T -> ref shared(T) or T* -> shared(T)*); worst you
 get is some extra synchronization.
shared(T)* is not shared itself, so that's fine, yeah.
The conversion T* -> shared(T)* isn't ok, of course. And it's correctly disallowed already. But it's for a different reason than this thread is about.
Jul 10 2016
prev sibling parent reply Kagamin <spam here.lot> writes:
On Monday, 11 July 2016 at 05:26:42 UTC, ag0aep6g wrote:
 Simply disallow reading and writing shared then, forcing 
 something more explicit like atomicLoad/atomicStore?

 That would be better than the current state, but it would make 
 shared even more unwieldy.
Atomic loads are only needed for volatile variables, not for all kinds of shared data. Also currently atomicLoad doesn't provide functionality equivalent to raw load.
 Generating atomic operations would break less code, and feels 
 like the obvious thing to me.
Multithreaded code can't be generated.
Jul 11 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/11/2016 03:31 PM, Kagamin wrote:
 Atomic loads are only needed for volatile variables, not for all kinds
 of shared data.
Volatile just means that another thread can mess with the data, right? So shared data that's not being written to from elsewhere isn't volatile, and one doesn't need an atomic load to read it. If I got that right: Sure. But the compiler can't know if a shared variable is volatile or not, so it has to assume that it is. If the programmer knows that it's not volatile, they can cast shared away and use a normal load.
 Also currently atomicLoad doesn't provide functionality
 equivalent to raw load.
Is a "raw load" just a non-atomic load, or is it something special? What's the relevance of atomicLoad's capabilities?
 Generating atomic operations would break less code, and feels like the
 obvious thing to me.
Multithreaded code can't be generated.
For primitive types, atomic loads and stores can be generated, no? It's clear that this doesn't make the code automatically thread-safe. It just guards against an easily made mistake. Like shared is supposed to do, as far as I understand.
Jul 11 2016
next sibling parent reply Kagamin <spam here.lot> writes:
On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:
 If I got that right: Sure. But the compiler can't know if a 
 shared variable is volatile or not, so it has to assume that it 
 is. If the programmer knows that it's not volatile, they can 
 cast shared away and use a normal load.
If you cast shared away, an unshared postblit will be called instead of shared one.
Jul 12 2016
parent ag0aep6g <anonymous example.com> writes:
On 07/12/2016 02:26 PM, Kagamin wrote:
 On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:
 If I got that right: Sure. But the compiler can't know if a shared
 variable is volatile or not, so it has to assume that it is. If the
 programmer knows that it's not volatile, they can cast shared away and
 use a normal load.
If you cast shared away, an unshared postblit will be called instead of shared one.
True. One would have to watch out for that, and maybe work around correspondingly. The cast would be a greppable indicator that special care is needed. In contrast, unsafe reading/writing of shared data that looks like just another assignment is similarly problematic and harder to spot. If reading/writing shared directly would be disallowed completely (always have to use core.atomic or cast), then shared postblits wouldn't make sense anymore, I guess.
Jul 12 2016
prev sibling parent reply Kagamin <spam here.lot> writes:
On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:
 Also currently atomicLoad doesn't provide functionality
 equivalent to raw load.
Is a "raw load" just a non-atomic load, or is it something special? What's the relevance of atomicLoad's capabilities?
You suggested to use atomicLoad to access shared data. Raw atomic load is an assembler instruction and can't be optimized.
Jul 12 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/12/2016 02:28 PM, Kagamin wrote:
 On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:
 Also currently atomicLoad doesn't provide functionality
 equivalent to raw load.
Is a "raw load" just a non-atomic load, or is it something special? What's the relevance of atomicLoad's capabilities?
You suggested to use atomicLoad to access shared data. Raw atomic load is an assembler instruction and can't be optimized.
Correct me if I'm wrong: Currently, the compiler generates a non-atomic load for reading shared data. In particular, it doesn't generate a raw atomic load. And you say you can't do a raw atomic load with core.atomic.atomicLoad. Then how does one do a raw atomic load at the moment? Does it become harder or impossible when the language is changed in one of the two suggested ways (generate another kind of atomic load / outright reject reading of shared types)?
Jul 12 2016
parent reply Kagamin <spam here.lot> writes:
On Tuesday, 12 July 2016 at 12:47:05 UTC, ag0aep6g wrote:
 In contrast, unsafe reading/writing of shared data that looks 
 like just another assignment is similarly problematic and 
 harder to spot.
It's strange if you want to expose a bare volatile variable to the rest of the world and expect the world to cope with concurrency on every access.
 If reading/writing shared directly would be disallowed 
 completely (always have to use core.atomic or cast), then 
 shared postblits wouldn't make sense anymore, I guess.
The problem is when an unshared postblit is called on the shared data.
 Correct me if I'm wrong: Currently, the compiler generates a 
 non-atomic load for reading shared data. In particular, it 
 doesn't generate a raw atomic load. And you say you can't do a 
 raw atomic load with core.atomic.atomicLoad.
I say non-atomic load is not supported by atomicLoad. 5th kind of synchronization will help here.
Jul 12 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/12/2016 03:51 PM, Kagamin wrote:
 It's strange if you want to expose a bare volatile variable to the rest
 of the world and expect the world to cope with concurrency on every access.
I'm not sure what you're saying here. Should unsafe reading and writing of shared types simply be allowed and have the common syntax (as it is now), and the programmer should make sure that things are set up correctly? That's a reasonable stance, but it's not the one that D takes towards shared when `shared int x; ++x` is deprecated.
 The problem is when an unshared postblit is called on the shared data.
I understand that. The programmer who writes the cast would have to work around it. I agree that it's rather ugly. Maybe a function can be written that does an unsafe copy and then calls the shared postblit.
 Correct me if I'm wrong: Currently, the compiler generates a
 non-atomic load for reading shared data. In particular, it doesn't
 generate a raw atomic load. And you say you can't do a raw atomic load
 with core.atomic.atomicLoad.
I say non-atomic load is not supported by atomicLoad. 5th kind of synchronization will help here.
So a "raw load" is just a non-atomic load after all? When I asked earlier you wrote "raw atomic load" in your reply, so I assumed it was some kind of atomic load. Supporting non-atomic load in atomicLoad would be weird, wouldn't it? It would be opposite of what it says on the label. Would the point be that it calls the shared postblit (unlike casting shared away and then copying)? As mentioned above, I'd hope that a function that does that can be written. I didn't manage to find out what the "5th kind of synchronization" is. If it's important for this discussion, please elaborate.
Jul 12 2016
parent reply Kagamin <spam here.lot> writes:
On Tuesday, 12 July 2016 at 14:38:27 UTC, ag0aep6g wrote:
 I'm not sure what you're saying here. Should unsafe reading and 
 writing of shared types simply be allowed and have the common 
 syntax (as it is now), and the programmer should make sure that 
 things are set up correctly?

 That's a reasonable stance, but it's not the one that D takes 
 towards shared when `shared int x; ++x` is deprecated.
shared only differentiates between shared and unshared data. Teaching people to write legit concurrent code is a different task. Increment of a shared variable doesn't have a compelling use case so whatever happens to it is not important, just storing shared data is more common.
 Supporting non-atomic load in atomicLoad would be weird, 
 wouldn't it? It would be opposite of what it says on the label.
The name can be different, e.g. fast load, which better reflects its purpose.
 Would the point be that it calls the shared postblit (unlike 
 casting shared away and then copying)? As mentioned above, I'd 
 hope that a function that does that can be written.
Postblit is a problem with casting. Performance is a problem with atomicLoad.
Jul 12 2016
parent reply ag0aep6g <anonymous example.com> writes:
On Tuesday, 12 July 2016 at 15:46:52 UTC, Kagamin wrote:
 shared only differentiates between shared and unshared data. 
 Teaching people to write legit concurrent code is a different 
 task. Increment of a shared variable doesn't have a compelling 
 use case so whatever happens to it is not important, just 
 storing shared data is more common.
The deprecation of ++ shows that (currently) shared's purpose is not only to differentiate between shared and unshared data. Either we go forward with the deprecation of ++. Then reads and writes should follow the same pattern. Or we should revert the deprecation of ++ and say that people must simply be careful with shared. The current inconsistency is no good.
Jul 12 2016
parent Kagamin <spam here.lot> writes:
On Tuesday, 12 July 2016 at 16:14:58 UTC, ag0aep6g wrote:
 The deprecation of ++ shows that (currently) shared's purpose 
 is not only to differentiate between shared and unshared data.
Feel free to submit a corresponding spec change PR stating that you want to extend shared's purpose to solve as many concurrency problems as possible at any cost and teach people to write concurrent code and have Walter review it.
 The current inconsistency is no good.
What we need is a shared FAQ with answers to common misunderstandings about it.
Jul 13 2016
prev sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 7/10/16 9:02 AM, ag0aep6g wrote:
 While messing with atomicLoad [1], I noticed that dmd lets me implicitly
 convert values to/from shared without restrictions. It's in the spec
 [2]. This seems bad to me.
I think you misunderstand the problem here. Conversion means changing the type. Once you have loaded the shared data into a register, or whatever, it's no longer shared, it's local. Writing it out to another place doesn't change anything. It's once you add references into the mix where you may have a problem. What I think you mean (and I think you realize this now), is that the actual copying of the data should not be implicitly allowed. The type change is fine, it's the physical reading or writing of shared data that can cause issues. I agree we should extend the rules to prevent this. In other words: shared int x; void main() { // ++x; // not allowed int x2 = x + 1; // but this is x = x2; // and it shouldn't be } -Steve
Jul 11 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/11/2016 02:54 PM, Steven Schveighoffer wrote:
 I think you misunderstand the problem here.
Yes.
 Conversion means changing
 the type.

 Once you have loaded the shared data into a register, or whatever, it's
 no longer shared, it's local. Writing it out to another place doesn't
 change anything. It's once you add references into the mix where you may
 have a problem.
Right.
 What I think you mean (and I think you realize this now), is that the
 actual copying of the data should not be implicitly allowed. The type
 change is fine, it's the physical reading or writing of shared data that
 can cause issues. I agree we should extend the rules to prevent this.
Exactly.
 In other words:

 shared int x;

 void main()
 {
     // ++x; // not allowed
     int x2 = x + 1; // but this is
     x = x2; // and it shouldn't be
 }
I think I would prefer if the compiler would generate atomic operations, but I'm clearly far from being an expert on any of this. Simply rejecting the code would be fine with me, too. Also: shared int x; shared int y; x = y; /* should be rejected too (or be atomic, if that's even possible) */
Jul 11 2016
parent ag0aep6g <anonymous example.com> writes:
On 07/11/2016 03:23 PM, ag0aep6g wrote:
 I think I would prefer if the compiler would generate atomic operations,
Backpedaling on that one. With automatic atomic loads and stores, one could accidentally write this: shared int x; x = x + 1; /* atomic load + atomic != atomic increment */ Easy to miss the problem, because the code looks so innocent. But when the atomic loads and stores must be spelled out it would look like in my original post: shared int x; atomicStore(x, atomicLoad(x) + 1); Way more obvious that the code isn't actually thread-safe. So now I'm leaning towards requiring the verbose version.
Jul 11 2016