assumeLocal: convert shared lvalue to a non-shared one by radcapricorn · Pull Request #724 · dlang/druntime

radcapricorn · 2014-02-15T06:15:01Z

This is one of the two functions to satisfy ER 12133 - Relaxed read-modify-write for shared lvalues.

ghost · 2014-02-15T10:39:50Z

src/core/atomic.d

Kill off the As the name suggests part please (we're aiming for objective documentation). :)

If this doesn't support classes it should document this. But why doesn't it support classes? That too should be documented.

MartinNowak · 2014-02-15T13:58:30Z

Something is wrong here. Why do you make something shared if it's protected by a mutex?
Atomic access without the mutex would become a failure if you use assumeLocal with the mutex.

radcapricorn · 2014-02-15T14:25:37Z

Consider member variables inside a shared class (i.e. Condition implementation for Windows). They're transitively shared. But if you only ever access such variables under a lock, you don't need explicit atomicOps at all.

It's true that accessing the variable outside of the lock will be erroneous, but this function is a convenience for cases when such access is not needed. See e.g. my ongoing branch implementing shared for all core.sync primitives.

Some kind of workaround is needed. Lots of casts or chains of raw atomicLoad/Store calls IMO are inferior to this little helper.

MartinNowak · 2014-02-15T15:52:27Z

What's with automatic HeadUnshared for synchronized classes?
Looks like it's not implemented (http://dpaste.dzfl.pl/3e4d6b59f06a).

I don't want to slow down you efforts to make core.sync shared and I think assumeLocal is a useful primitive.
But please note that the shared concept is still fuzzy. As Andrei mentioned we need to finish the specs and implementation.
It would be great if you could help with that.

radcapricorn · 2014-02-15T21:56:06Z

What's with automatic HeadUnshared for synchronized classes?

Even if it were implemented in that exact wording, it would be of little help in many cases. I know that in the grand scheme, synchronized is supposed to be used for lock-based synchronization and shared - for lock-free. But that is a very restrictive generalization. For example, making Semaphores or Conditions synchronized classes would be an overkill... no, an error (deadlocks), yet they should support shared (which they currently don't). Additionally, I bet there will be cases when using classes is not an option but some form of synchronization is needed.

But please note that the shared concept is still fuzzy. As Andrei mentioned we need to finish the specs and implementation.

I know, that is my thread :)

It would be great if you could help with that.

I'm trying to.

andralex · 2014-03-16T03:27:04Z

src/core/atomic.d

if (!is(T == class))

andralex · 2014-03-16T03:38:21Z

Fine, but let's call it assumeThreadLocal because one may think it has to do with scopes. Please follow the phobos conventions in spacing.

andralex · 2014-03-16T04:00:57Z

Take that back, should be assumeUnshared because that's the actual assumption.

MartinNowak · 2014-03-28T19:54:24Z

ping

dnadlinger · 2014-03-28T20:01:42Z

+1 for assumeUnshared.

radcapricorn · 2014-07-04T15:09:40Z

I apologize for so long an absence, got caught up in a no-D world. So, rename to assumeUnshared, remove @trusted, rebase, commit?

andralex · 2014-09-23T14:51:37Z

yes please

MartinNowak · 2014-11-23T01:16:40Z

Ping

DmitryOlshansky · 2015-07-15T21:25:10Z

@radcapricorn any chance to revive this? Seems like non-controversial stuff that just fell off the radar.

DmitryOlshansky · 2016-03-09T16:30:53Z

Ping @radcapricorn ?

radcapricorn · 2017-04-12T22:58:48Z

Three years ><

andralex · 2017-04-12T23:15:32Z

src/core/atomic.d

+    ref T assumeUnshared(T)(ref shared T val) @system @nogc pure nothrow
+        if(!is(T == class) && !is(T == interface))
+    {
+        return *cast(T*)&val;


space after close paren in cast

andralex · 2017-04-12T23:16:27Z

src/core/atomic.d

+     * Returns:
+     *  The non-shared lvalue.
+     */
+    ref immutable(T) assumeUnshared(T)(ref immutable(T) val) @safe @nogc pure nothrow


What would be a use case for this? It doesn't change the type of its argument.

Generic code.

shared struct Example(T) { private T payload; T getUnsharedCopyOfPayload() const { return payload.assumeUnshared; } } auto createExample(T)(auto ref T value) { return shared(Example!T)(value); } immutable int i = 42; auto example = createExample(i); assert(is(typeof(example.getUnsharedCopyOfPayload()) == immutable(int));

This way, Example doesn't have to specialize such use case.

IOTW, it's safe to assume immutable as unshared. But the signature ref T assumeUnshared(T)(ref shared T val) will not accept immutable lvalue, hence the specialization.

immutable is actually implicitly shared

Exactly, and assumeUnshared should accept shared arguments, therefore it shouldn't fail to compile with immutable argument. It works as advertised: takes shared lvalue and yields lvalue that can be treated as unshared. It just so happens that immutable is both at the same time.

@andralex @radcapricorn anything else still not clear?

I'm still unconvinced about the usefulness. By that logic we should also define assumeUnshared for regular data so assumeUnshared works transparently with all data types (after all unshared data can be assumed to be unshared!)

One possibility is to not introduce this overload yet, and see how necessary it is in practice.

At any rate, if we do introduce the overload the documentation must be merged with the other overload's.

andralex

This is a good addition. A few nits need fixing.

andralex · 2017-09-07T21:21:45Z

src/core/atomic.d

+    ref T assumeUnshared(T)(ref shared T val) @system @nogc pure nothrow
+        if(!is(T == class) && !is(T == interface))
+    {
+        return *cast(T*) &val;


This is the ddoc version, right? Then it should not have an implementation.

andralex · 2017-09-07T21:23:56Z

src/core/atomic.d

+     * Returns:
+     *  The non-shared lvalue.
+     */
+    ref immutable(T) assumeUnshared(T)(ref immutable(T) val) @safe @nogc pure nothrow


I'm still unconvinced about the usefulness. By that logic we should also define assumeUnshared for regular data so assumeUnshared works transparently with all data types (after all unshared data can be assumed to be unshared!)

One possibility is to not introduce this overload yet, and see how necessary it is in practice.

At any rate, if we do introduce the overload the documentation must be merged with the other overload's.

andralex · 2017-09-07T21:24:20Z

src/core/atomic.d


+// assumeUnshared is architecture-independent: it is just a cast
+ref auto assumeUnshared(T)(ref shared T val) @system @nogc pure nothrow
+    if(!is(T == class) && !is(T == interface))


obey phobos, if flush to the left and space after it

wilzbach · 2018-03-29T00:06:04Z

Revived this to #2156

ghost reviewed Feb 15, 2014
View reviewed changes

radcapricorn added 3 commits February 16, 2014 09:12

assumeLocal: convert shared lvalue to a non-shared one

664fadf

Fixed documentation

ac3cccb

More doc fixes; stray whitespace

eaab617

andralex reviewed Mar 16, 2014
View reviewed changes

src/core/atomic.d Outdated

Copy link

Member

andralex Mar 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!is(T == class))

Merge remote-tracking branch 'upstream/master' into assume_local

c51955e

Small doc fix

edb108b

andralex reviewed Apr 12, 2017

View reviewed changes

Space after close paren in cast

b2fc618

andralex approved these changes Sep 7, 2017

View reviewed changes

dlang-bot added Needs Rebase needs a `git rebase` performed stalled Needs Work labels Jan 1, 2018

wilzbach mentioned this pull request Mar 29, 2018

assumeUnshared: convert shared lvalue to a non-shared one #2156

Open

wilzbach closed this Mar 29, 2018

Uh oh!

Conversation

radcapricorn commented Feb 15, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinNowak commented Feb 15, 2014

Uh oh!

radcapricorn commented Feb 15, 2014

Uh oh!

MartinNowak commented Feb 15, 2014

Uh oh!

radcapricorn commented Feb 15, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex commented Mar 16, 2014

Uh oh!

andralex commented Mar 16, 2014

Uh oh!

MartinNowak commented Mar 28, 2014

Uh oh!

dnadlinger commented Mar 28, 2014

Uh oh!

radcapricorn commented Jul 4, 2014

Uh oh!

andralex commented Sep 23, 2014

Uh oh!

MartinNowak commented Nov 23, 2014

Uh oh!

DmitryOlshansky commented Jul 15, 2015

Uh oh!

DmitryOlshansky commented Mar 9, 2016

Uh oh!

radcapricorn commented Apr 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radcapricorn Apr 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex Sep 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex Sep 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Mar 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Comments

radcapricorn Apr 12, 2017 •

edited

Loading

andralex Sep 7, 2017 •

edited

Loading

andralex Sep 7, 2017 •

edited

Loading