This repository has been archived by the owner on Oct 12, 2022. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 426
assumeLocal: convert shared lvalue to a non-shared one #724
Closed
Closed
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
664fadf
assumeLocal: convert shared lvalue to a non-shared one
radcapricorn ac3cccb
Fixed documentation
radcapricorn eaab617
More doc fixes; stray whitespace
radcapricorn c51955e
Merge remote-tracking branch 'upstream/master' into assume_local
radcapricorn edb108b
Small doc fix
radcapricorn b2fc618
Space after close paren in cast
radcapricorn File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -151,6 +151,52 @@ version( CoreDdoc ) | |
* loads and stores after the call. | ||
*/ | ||
void atomicFence() nothrow; | ||
|
||
/** | ||
* Converts a shared lvalue to a non-shared lvalue. | ||
* | ||
* This functions allows to treat a shared lvalue as if it was thread-local. | ||
* It is useful to avoid overhead of atomic operations when access to shared data | ||
* is known to be within one thread (i.e. always under a lock). | ||
* --- | ||
* shared static int i; | ||
* | ||
* // i is never used outside of synchronized {} blocks... | ||
* | ||
* synchronized | ||
* { | ||
* ++i; // ERROR: cannot directly modify shared lvalue | ||
* | ||
* atomicOp!"+="(i, 1); // possible overhead | ||
* | ||
* // Directly modify i | ||
* assumeLocal(i) += 1; | ||
* // or: | ||
* ++assumeLocal(i); | ||
* // or: | ||
* i.assumeLocal += 1; | ||
* } | ||
* --- | ||
* Usage of this function is restricted to allowing limited lvalue access to shared instances of | ||
* primitive and POD types (e.g. direct use of operators), thus it is not defined for classes. | ||
* | ||
* Note: this function does not perform any ordering. | ||
* | ||
* Note: assumeLocal is a special-purpose primitive and should be used with care. When accessing | ||
* shared variables both inside and outside of synchronized blocks, atomic operations should be | ||
* used instead. | ||
* | ||
* Params: | ||
* val = the shared lvalue. | ||
* | ||
* Returns: | ||
* The non-shared lvalue. | ||
*/ | ||
ref T assumeLocal(T)( ref shared T val ) @trusted pure nothrow | ||
if( !is( T == class ) ) | ||
{ | ||
return *cast(T*)&val; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. space after close paren in cast |
||
} | ||
} | ||
else version( AsmX86_32 ) | ||
{ | ||
|
@@ -1090,6 +1136,13 @@ if(__traits(isFloating, T)) | |
} | ||
} | ||
|
||
// assumeLocal is architecture-independent: it is just a cast | ||
ref auto assumeLocal(T)( ref shared T val ) @trusted pure nothrow | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see how this can be trusted |
||
if( !is( T == class ) ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
return *cast(T*)&val; | ||
} | ||
|
||
//////////////////////////////////////////////////////////////////////////////// | ||
// Unit Tests | ||
//////////////////////////////////////////////////////////////////////////////// | ||
|
@@ -1246,4 +1299,25 @@ version( unittest ) | |
|
||
assert(*r == 42); | ||
} | ||
|
||
unittest | ||
{ | ||
int base = 0; | ||
shared int atom = 0; | ||
|
||
// only accept shared lvalues | ||
static assert(!__traits(compiles, assumeLocal(base))); | ||
static assert(!__traits(compiles, assumeLocal(cast(shared)base))); | ||
|
||
++assumeLocal(atom); | ||
assert(atomicLoad!(MemoryOrder.raw)(atom) == 1); | ||
|
||
static class Klass {} | ||
auto c1 = new Klass; | ||
auto c2 = new shared Klass; | ||
|
||
// don't accept class instances | ||
static assert(!__traits(compiles, assumeLocal(c1))); | ||
static assert(!__traits(compiles, assumeLocal(c2))); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also write
i.assumeLocal += 1
. Whatever floats one's boat.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, to be updated then :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, regarding documentation, should I add myself in Authors block? Or put this block in function documentation? Or?..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, git history has made that practice obsolete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. thanks. Docs updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, it's a good idea to add yourself to the authors list if you've contributed a fairly large amount of code to a module and you're willing to take on a sort of maintainer role for at least the code that you've contributed. Git history isn't helpful for that.