www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - I want to add a Phobos module with template mixins for common idioms.

reply "Idan Arye" <GenericNPC gmail.com> writes:
Template mixins can be used to implement some common programming 
idioms(and design patterns). I have already implemented two of 
them for my own project - and I want to make them into a Phobos 
module - maybe `std.mixins`. The wiki at 
http://wiki.dlang.org/Review/Process says that before I start a 
new module contribution, I should post here to see if the 
community actually wants it, so this is what I'm doing.

The first one is the property idiom - a pair of getter and setter 
methods used to mutate a private member field. I have created 
four variants:
`propertyGetterOnly` - creates only the field and the getter.
`property` - creates the field, a getter and a setter.
`propertyAsserted` - creates the field, a getter and a setter 
that verifies it's input using an assert.
`propertyVerified` - creates the field, a getter and a setter 
that verifies it's input using an `if` statement and throws 
`PropertyException` if you try setting it to a bad value.

For example:

     class Foo
     {
         //Add an `int` property with value of 44, that can not 
exceed 60.
         mixin propertyVerified!(int, "x", `a <= 60`, 44);
     }

     Foo foo = new Foo();
     writeln(foo.x); //Prints 44
     foo.x = 55;
     writeln(foo.x); //Prints 55
     foo.x = 66; //throws PropertyException


The second idiom is the singleton pattern. I implemented it using 
the check-lock-check version, but I want to change the 
implementation to Dave's and Alexander Terekhov's low lock 
version(http://forum.dlang.org/thread/pelhvaxwjzhehdjtpsav forum.dlang.org). 
At any rate, it'll be used like this

     class Foo
     {
         mixin lowLockSingleton!false;

         private this(/*args*/)
         {
             ...
         }

         void init(/*args*/)
         {
             singleton_initInstance(new Foo(/*args*/));
         }
     }

The `false` sent to `lowLockSingleton` means you don't want to 
automatically initialize your singleton when someone request an 
instance before it was initialized - this is useful if you want 
to send initialization arguments. You could set it to 
`true`(which will be the default) and get a regular singleton, 
but I want to elaborate on this version. `lowLockSingleton` 
creates a private static method named `singleton_initInstance` 
which accepts a lazy argument for constructing the global 
instance. You need to implement your own initialization method, 
which should call `singleton_initInstance` to set the global 
instance based on initialization arguments. The 
`lowLockSingleton` implementation will make sure that only one 
call to `singleton_initInstance` will invoke the lazy argument 
and set the global instance to it's result.

I also want to implement a thread local singleton. That's it for 
now, but I think that's enough to justify a new module and once 
this module is up and running I'm sure other people will have 
other common and why-the-heck-is-this-not-common idioms to add.



The rationale behind having those template mixin is laziness. I'm 
a lazy programmer, and I know I'm not the only one.

I *know* I should use properties, and I know why, but it's so 
much easier to just use public fields. If I'll need getter&setter 
properties I can always change it later for just the fields where 
I need it, and if someone else needs them than it's their 
problem, not mine.

I *know* I should make my singletons thread-safe, and I know how, 
but the unsafe version is easier and between us, what are the 
chances for a race condition? *wink wink*

Having this `std.mixins` will make the right way also be the easy 
way. Writing `mixin property!(int, "x");` is not that bad, and 
since I can easily throw in a value check - I might just as well 
do it! And while the unsafe singleton is easier to implement than 
the safe version - not to mention the low lock version(they are 
not really *that* hard, but still harder than the unsafe version) 
- using `mixin lowLockSingleton;` is so much easier.



So, I would like to hear opinions. I already have a crude 
implementation of those two idioms, but I need to polish it some 
more before it's Phobos-worthy.

I also have a question about the contribution process. I know 
that when you contribute a small enhancement you need to open an 
enhancement issue on Bugzilla, but the modules in the review 
queue don't seem to have one. Are enhancement issues not required 
for a module contribution? Is this forum thread used instead of 
the Bugzilla issue to discuss the contribution?
Also, the wiki says that big features should start on their own 
feature 
branch(http://wiki.dlang.org/Development_and_Release_Process
Official_branches), 
but the modules on the review queue that have a GitHub pull 
request are pull-requested directly to master. Does that mean 
that what the wiki says is obsolete, or that the fork that these 
modules were developed on is in fact the feature branch?


Thanks for suffering through my long post!
May 06 2013
next sibling parent reply "Diggory" <diggsey googlemail.com> writes:
It's a nice idea but personally I don't like the syntax much, for 
example it's completely non-obvious what "true" does when passed 
to the singleton mixin, or that the parameters to the property 
mixin are "type, name, condition, initial value".

I suppose you could do something like this:
mixin property!`int x = 1`;

The other problem is that I don't think it's beneficial to invite 
the use of mixins for such simple substitutions. I'd rather see 
the majority of code be standard D syntax, and the use of mixins 
be the exception rather than the rule. It's similar to how 
excessive use of macros in C++ is generally considered bad 
practice.
May 06 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 6 May 2013 at 19:47:33 UTC, Diggory wrote:

 It's a nice idea but personally I don't like the syntax much, 
 for example it's completely non-obvious what "true" does when 
 passed to the singleton mixin, or that the parameters to the 
 property mixin are "type, name, condition, initial value".

 I suppose you could do something like this:
 mixin property!`int x = 1`;

 The other problem is that I don't think it's beneficial to 
 invite the use of mixins for such simple substitutions. I'd 
 rather see the majority of code be standard D syntax, and the 
 use of mixins be the exception rather than the rule. It's 
 similar to how excessive use of macros in C++ is generally 
 considered bad practice.


If D supported Java's annotation metaprogramming I could have 
implemented a syntax like:

      Property(`a < 50`) int a = 1;

since it doesn't, I have to use mixin templates...

(Declaimer: I do not want to imply here that Java is a good 
language. Java is a terrible language. Annotation metaprogramming 
is it's only redeeming feature, and even that feature is far too 
complex for common use...)


Anyways, I'll have to add a new trait template to extract the 
name from the declaration(Compile time regex are too slow), but I 
think I can pull off the style you suggest. However, we still 
need an extra template argument for the verifier, something like 
this:

     mixin propertyVerified!(`int x = 1`, `a < 50`);

Is this readable enough?
May 06 2013
next sibling parent "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 6 May 2013 at 21:18:56 UTC, Idan Arye wrote:

 Anyways, I'll have to add a new trait template to extract the 
 name from the declaration(Compile time regex are too slow)


You know what? I take that back. I can probably pull this off 
with an anonymous struct to get better compilation speed *and* 
more readable code.
May 06 2013
prev sibling parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Mon, 06 May 2013 17:13:00 -0400, Idan Arye <GenericNPC gmail.com> wrote:


 On Monday, 6 May 2013 at 19:47:33 UTC, Diggory wrote:

 It's a nice idea but personally I don't like the syntax much, for  
 example it's completely non-obvious what "true" does when passed to the  
 singleton mixin, or that the parameters to the property mixin are  
 "type, name, condition, initial value".

 I suppose you could do something like this:
 mixin property!`int x = 1`;

 The other problem is that I don't think it's beneficial to invite the  
 use of mixins for such simple substitutions. I'd rather see the  
 majority of code be standard D syntax, and the use of mixins be the  
 exception rather than the rule. It's similar to how excessive use of  
 macros in C++ is generally considered bad practice.


 If D supported Java's annotation metaprogramming I could have  
 implemented a syntax like:

       Property(`a < 50`) int a = 1;

 since it doesn't, I have to use mixin templates...


D supports  Property syntax (though I would suggest a different  
identifier) via User-defined-attributes.  However, you will still need the  
mixin, I don't think it can add code.

This brings up a good idiom though.  Since mixin is the only thing that  
can inject code, but its usage is ugly,  uda's can be useful for  
specifying nice parameters to the mixin, and you could have one mixin per  
class instead of per property.  I think Manu uses this to great effect  
(was in his first talk).

-Steve
May 06 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 6 May 2013 at 21:58:08 UTC, Steven Schveighoffer wrote:

 On Mon, 06 May 2013 17:13:00 -0400, Idan Arye 
 <GenericNPC gmail.com> wrote:


 On Monday, 6 May 2013 at 19:47:33 UTC, Diggory wrote:

 It's a nice idea but personally I don't like the syntax much, 
 for example it's completely non-obvious what "true" does when 
 passed to the singleton mixin, or that the parameters to the 
 property mixin are "type, name, condition, initial value".

 I suppose you could do something like this:
 mixin property!`int x = 1`;

 The other problem is that I don't think it's beneficial to 
 invite the use of mixins for such simple substitutions. I'd 
 rather see the majority of code be standard D syntax, and the 
 use of mixins be the exception rather than the rule. It's 
 similar to how excessive use of macros in C++ is generally 
 considered bad practice.


 If D supported Java's annotation metaprogramming I could have 
 implemented a syntax like:

      Property(`a < 50`) int a = 1;

 since it doesn't, I have to use mixin templates...


 D supports  Property syntax (though I would suggest a different 
 identifier) via User-defined-attributes.  However, you will 
 still need the mixin, I don't think it can add code.

 This brings up a good idiom though.  Since mixin is the only 
 thing that can inject code, but its usage is ugly,  uda's can 
 be useful for specifying nice parameters to the mixin, and you 
 could have one mixin per class instead of per property.  I 
 think Manu uses this to great effect (was in his first talk).

 -Steve


I don't like this idea of marking the class with a `mixin` 
statement, and having all the parameters fed to it somewhere 
else. It doesn't feel clean to me...

And feeling aside, there was one thing I neglected - in Java the 
convention is to use `getX` and `setX` as `x`'s accessors, but in 
D the convention is to use ` property` accessors. Now, if we 
wrote:

     class Foo
     {
         private  Property int x;
         mixin makeProperties;
     }

then `makeProperties` would have made:

      property int x(){...}
      property int x(int value){...}

And we wouldn't be able to use them because they are overshadowed 
by the original x! That means we would have to rename `x` to 
`m_x` and either supply the desired property name as a UDA 
attribute or analyze the variable's name to remove the prefix 
notation.

Not as elegant as we wanted.


However, your idea of having a single mixin handle all the 
class\struct's properties gave me another idea - to use a single 
mixin, but instead of having it analyze the owner class to find 
the fields we want to make properties, we simply give them to it 
with a token string:

     class Foo
     {
         mixin properties!q{
              asserting(`a < 50`)     int x = 1;
              verifying(`a !is null`) string y = "hello";
              privateSetter           double z = 44.4;
         };
     }

This can be easily implemented by making a private `struct` and 
`mixin`ing all those declarations as it's arguments, which gives 
me a data structure schema to query with `std.traits`, and also 
allows me to define the UDA's so that they can only be used 
inside the `mixin properties` "block". Also, it encourages 
programmers to put all their member fields in the same section of 
the class\struct.

One problem I see here, BTW, is DDoc - I have no idea how to 
document properties created like this...
May 06 2013
parent Jacob Carlborg <doob me.com> writes:
On 2013-05-07 01:20, Idan Arye wrote:


 However, your idea of having a single mixin handle all the
 class\struct's properties gave me another idea - to use a single mixin,
 but instead of having it analyze the owner class to find the fields we
 want to make properties, we simply give them to it with a token string:

      class Foo
      {
          mixin properties!q{
               asserting(`a < 50`)     int x = 1;
               verifying(`a !is null`) string y = "hello";
               privateSetter           double z = 44.4;
          };
      }


Please no strings, it's horrible.

-- 
/Jacob Carlborg
May 06 2013
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 5/6/13 3:14 PM, Idan Arye wrote:

 Template mixins can be used to implement some common programming
 idioms(and design patterns). I have already implemented two of them for
 my own project - and I want to make them into a Phobos module - maybe
 `std.mixins`.


Nice idea, but at the first level of detail (without looking into it) 
the title should not be dictated by implementation artifacts, i.e. we 
don't have modules such as "std.templates" or "std.structs" etc. 
Conversely, if you implement a pattern via other means than a mixin, 
you're in trouble :o).

Andrei
May 06 2013
parent reply "Robert Rouse" <robert.e.rouse gmail.com> writes:
On Monday, 6 May 2013 at 19:57:27 UTC, Andrei Alexandrescu wrote:

 On 5/6/13 3:14 PM, Idan Arye wrote:

 Template mixins can be used to implement some common 
 programming
 idioms(and design patterns). I have already implemented two of 
 them for
 my own project - and I want to make them into a Phobos module 
 - maybe
 `std.mixins`.


 Nice idea, but at the first level of detail (without looking 
 into it) the title should not be dictated by implementation 
 artifacts, i.e. we don't have modules such as "std.templates" 
 or "std.structs" etc. Conversely, if you implement a pattern 
 via other means than a mixin, you're in trouble :o).

 Andrei


Since they are idioms, why not std.idioms or std.idiomatic?
May 06 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 5/6/13 4:03 PM, Robert Rouse wrote:

 On Monday, 6 May 2013 at 19:57:27 UTC, Andrei Alexandrescu wrote:

 On 5/6/13 3:14 PM, Idan Arye wrote:

 Template mixins can be used to implement some common programming
 idioms(and design patterns). I have already implemented two of them for
 my own project - and I want to make them into a Phobos module - maybe
 `std.mixins`.


 Nice idea, but at the first level of detail (without looking into it)
 the title should not be dictated by implementation artifacts, i.e. we
 don't have modules such as "std.templates" or "std.structs" etc.
 Conversely, if you implement a pattern via other means than a mixin,
 you're in trouble :o).

 Andrei


 Since they are idioms, why not std.idioms or std.idiomatic?


std.patterns is the sweet spot.

Andrei
May 06 2013
parent "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 6 May 2013 at 20:10:59 UTC, Andrei Alexandrescu wrote:

 On 5/6/13 4:03 PM, Robert Rouse wrote:

 On Monday, 6 May 2013 at 19:57:27 UTC, Andrei Alexandrescu 
 wrote:

 On 5/6/13 3:14 PM, Idan Arye wrote:

 Template mixins can be used to implement some common 
 programming
 idioms(and design patterns). I have already implemented two 
 of them for
 my own project - and I want to make them into a Phobos 
 module - maybe
 `std.mixins`.


 Nice idea, but at the first level of detail (without looking 
 into it)
 the title should not be dictated by implementation artifacts, 
 i.e. we
 don't have modules such as "std.templates" or "std.structs" 
 etc.
 Conversely, if you implement a pattern via other means than a 
 mixin,
 you're in trouble :o).

 Andrei


 Since they are idioms, why not std.idioms or std.idiomatic?


 std.patterns is the sweet spot.

 Andrei


I like `std.idioms`. `std.patterns` can be ambiguous, since it 
can also refer to regex patterns or to functional pattern 
matching - and not just to design patterns.
May 06 2013
prev sibling next sibling parent reply "Dicebot" <m.strashun gmail.com> writes:
I really think that UDA annotations + single mixin like this:

class Test
{
     private:
         // annotated stuff
     public:
     mixin implementAnnotations!Test;
}

is a most type-safe approach which scales.
May 07 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Tuesday, 7 May 2013 at 08:58:47 UTC, Dicebot wrote:

 I really think that UDA annotations + single mixin like this:

 class Test
 {
     private:
         // annotated stuff
     public:
     mixin implementAnnotations!Test;
 }

 is a most type-safe approach which scales.


I think any method will scale(except for insane methods like 
inheriting from a templated class that receives the properties 
schema via template arguments...). After all - we are dealing 
with declarations here, and we can't stuck as many as we want 
while still keeping the code readable.

The problem with any solution that declares the fields directly 
in the struct\class's body is that the field identifier 
overshadows the property methods. If we look at your example, and 
declare a field to be made into a property:

     class Test
     {
         private:
              Property int foo;
         public:
             mixin implementAnnotations!Test;
     }

Now, the expected behavior is that `implementAnnotations!Test` 
will declare a getter and a setter, both named `foo`. But even if 
it does that, it won't do us any good - because the accessors are 
declared inside a mixin, so they are overshadowed by the same 
`foo` we wanted to encapsulate!

So, we have to change it:

     class Test
     {
         private:
              Property int m_foo;
         public:
             mixin implementAnnotations!Test;
     }

and inside `implementAnnotations` we would have to strip `m_foo` 
out of it's prefix to get the getter&setter names.


If we send a token string to the mixin template, we can avoid 
this problem:

     class Test
     {
         public:
             mixin properties!q{
                 int foo;
             }
     }

And what happens inside the `properties` mixin is:

     mixin template properties(string declaration){
         private struct __Fields
         {
             mixin(declaration);
         }
         private __Fields __fields;
         // Create accessors properties for __fields's members.
         // Possibly create "m_*" aliases for __fields's members.
     }

Now the `foo` is not declared directly inside `Test` - instead it 
is declared inside `Test.__Fields` and accessed via the 
`__fields` struct. That means we are free to name our accessor 
property methods `foo` - since that identifier is still available 
in the main namespace of `Test`.

This solution has two problems(beside "x is evil" mantras):

1) Since the properties are created inside a struct, trying to 
probe `Test`(like `tupleof` or `std.traits.FieldTypeTuple`) will 
yield weird results. I believe I can solve this with a slightly 
more complex approach - instead of creating a member instance of 
`__Fields`, I can probe it myself and create `m_*` variations of 
the properties with the same types, storage classes etc.

2) If you use `typeof(this)` while declaring a property, it will 
yield `__Fields` instead of `Test`. I don't really know how to 
solve this, but this is an edge case - `typeof(this)` is usually 
used in mixins or templated methods, not in field declaration - 
so even with this problem, the mixin+string version is better 
suited for the common use-case.

Anyways, a small enhancement to dmd can help solve both problems 
easily - a mixin template that has an identifier and does not 
leak to the surrounding scope. But even without it - I can solve 
the first problem and the second problem concerns a rare 
use-case, so this is still a good solution if we want to declare 
the properties with the same names we are going to use them 
with...
May 07 2013
parent reply "Dicebot" <m.strashun gmail.com> writes:
On Tuesday, 7 May 2013 at 19:10:23 UTC, Idan Arye wrote:

 So, we have to change it:

     class Test
     {
         private:
              Property int m_foo;
         public:
             mixin implementAnnotations!Test;
     }

 and inside `implementAnnotations` we would have to strip 
 `m_foo` out of it's prefix to get the getter&setter names.


I believe it is exactly how it should be. With changeable 
compile-time naming convention function.
May 07 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Tuesday, 7 May 2013 at 19:10:23 UTC, Idan Arye wrote:

 Anyways, a small enhancement to dmd can help solve both 
 problems easily - a mixin template that has an identifier and 
 does not leak to the surrounding scope.


Actually, forget about that enhancement - I can use aliases, CTFE 
and a mixin to solve that problem.

On Tuesday, 7 May 2013 at 20:25:21 UTC, Dicebot wrote:

 On Tuesday, 7 May 2013 at 19:10:23 UTC, Idan Arye wrote:

 So, we have to change it:

    class Test
    {
        private:
             Property int m_foo;
        public:
            mixin implementAnnotations!Test;
    }

 and inside `implementAnnotations` we would have to strip 
 `m_foo` out of it's prefix to get the getter&setter names.


 I believe it is exactly how it should be. With changeable 
 compile-time naming convention function.


So this is where we disagree. I believe that if you want a 
property named `foo` you should be able to just name it `foo` to 
begin with.
May 07 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
OK, so I'm gonna go ahead and implement it, so I can show by 
example that the string solution can be typesafe, scalable and 
elegant.
May 08 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Wednesday, 8 May 2013 at 20:11:34 UTC, Idan Arye wrote:

 OK, so I'm gonna go ahead and implement it, so I can show by 
 example that the string solution can be typesafe, scalable and 
 elegant.


OK, this is a basic implementation:
https://gist.github.com/someboddy/5557358

Before I can make the pull request, I still need to do 
documentation, add some asserts to make sure users don't declare 
methods or subtypes in the property declarations string, add some 
more unit tests, and add the other idiom(the singleton).

But, it's still enough for demonstrating that strings are not 
evil, and that their usage here does not brake type safety, 
scope, or anything else.
May 10 2013
next sibling parent "Diggory" <diggsey googlemail.com> writes:
On Friday, 10 May 2013 at 21:04:32 UTC, Idan Arye wrote:

 On Wednesday, 8 May 2013 at 20:11:34 UTC, Idan Arye wrote:

 OK, so I'm gonna go ahead and implement it, so I can show by 
 example that the string solution can be typesafe, scalable and 
 elegant.


 OK, this is a basic implementation:
 https://gist.github.com/someboddy/5557358

 Before I can make the pull request, I still need to do 
 documentation, add some asserts to make sure users don't 
 declare methods or subtypes in the property declarations 
 string, add some more unit tests, and add the other idiom(the 
 singleton).

 But, it's still enough for demonstrating that strings are not 
 evil, and that their usage here does not brake type safety, 
 scope, or anything else.


That's a very nice syntax. It would be a good start for a mixin 
library.
May 10 2013
prev sibling parent "Tove" <tove fransson.se> writes:
On Friday, 10 May 2013 at 21:04:32 UTC, Idan Arye wrote:

 On Wednesday, 8 May 2013 at 20:11:34 UTC, Idan Arye wrote:

 OK, so I'm gonna go ahead and implement it, so I can show by 
 example that the string solution can be typesafe, scalable and 
 elegant.


 OK, this is a basic implementation:
 https://gist.github.com/someboddy/5557358

 Before I can make the pull request, I still need to do 
 documentation, add some asserts to make sure users don't 
 declare methods or subtypes in the property declarations 
 string, add some more unit tests, and add the other idiom(the 
 singleton).

 But, it's still enough for demonstrating that strings are not 
 evil, and that their usage here does not brake type safety, 
 scope, or anything else.


kickass technique, hope this get included soon, keep up the good 
work!
May 13 2013
prev sibling parent reply "Idan Arye" <GenericNPC gmail.com> writes:
OK, I implemented everything and made a pull request: 
https://github.com/D-Programming-Language/phobos/pull/1294
May 18 2013
next sibling parent reply "Diggory" <diggsey googlemail.com> writes:
On Saturday, 18 May 2013 at 16:58:19 UTC, Idan Arye wrote:

 OK, I implemented everything and made a pull request: 
 https://github.com/D-Programming-Language/phobos/pull/1294


Nice, but the singleton implementation seems somewhat 
over-complicated, and the low-lock singleton is broken, possibly 
as a result of the former problem.

You need two variables of type T (assuming T is the target 
class). One should be __gshared, the other thread-local. When 
"instance()" is called, first check if the thread-local variable 
is non-null, if so just return it. Otherwise enter a synchronized 
block, check if the __gshared variables is null (and if so 
initialise it) then copy its value to the thread-local variable 
and finally return it.

For a "hasInstance" method to make any sense, the caller must be 
able to synchronize on the same object that "instance()" uses to 
synchronize on when it accesses the __gshared variable. Otherwise 
the return value is out of date before it's even been returned.
May 19 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Sunday, 19 May 2013 at 08:36:24 UTC, Diggory wrote:

 On Saturday, 18 May 2013 at 16:58:19 UTC, Idan Arye wrote:

 OK, I implemented everything and made a pull request: 
 https://github.com/D-Programming-Language/phobos/pull/1294


 Nice, but the singleton implementation seems somewhat 
 over-complicated, and the low-lock singleton is broken, 
 possibly as a result of the former problem.

 You need two variables of type T (assuming T is the target 
 class). One should be __gshared, the other thread-local. When 
 "instance()" is called, first check if the thread-local 
 variable is non-null, if so just return it. Otherwise enter a 
 synchronized block, check if the __gshared variables is null 
 (and if so initialise it) then copy its value to the 
 thread-local variable and finally return it.

 For a "hasInstance" method to make any sense, the caller must 
 be able to synchronize on the same object that "instance()" 
 uses to synchronize on when it accesses the __gshared variable. 
 Otherwise the return value is out of date before it's even been 
 returned.


There is no point in saving a thread local reference to the 
global instance. The `__gshared` instance is never changed once 
initialized, so if we saved a thread local reference, it would 
*always* be either null or the same as the `__gshared` one - 
which means that if the local reference is not null, there is no 
difference between returning the local and the global references.

`hasInstance` does not need no synchronization - it would just 
slow it down. Synchronization is redundant in readonly and 
writeonly scenarios - and this is a readonly scenario. A single 
read is atomic with or without a synchronization.

At any rate, using my implementation was broekn - I forgot to set 
the thread local boolean instantiation indicator to true(which 
would mean there will always be a lock!). I fixed it. Thanks for 
pointing that out!
May 19 2013
parent reply "Diggory" <diggsey googlemail.com> writes:
 There is no point in saving a thread local reference to the 
 global instance. The `__gshared` instance is never changed once 
 initialized, so if we saved a thread local reference, it would 
 *always* be either null or the same as the `__gshared` one - 
 which means that if the local reference is not null, there is 
 no difference between returning the local and the global 
 references.

 `hasInstance` does not need no synchronization - it would just 
 slow it down. Synchronization is redundant in readonly and 
 writeonly scenarios - and this is a readonly scenario. A single 
 read is atomic with or without a synchronization.

 At any rate, using my implementation was broekn - I forgot to 
 set the thread local boolean instantiation indicator to 
 true(which would mean there will always be a lock!). I fixed 
 it. Thanks for pointing that out!


With regard to using a boolean instead of storing the instance 
thread locally - you're still reading from a mutable __gshared 
variable with no synchronisation on the reader's side, and that 
is always a bug. It may work in most cases but due to instruction 
reordering and differences between architectures there's no 
guarantee of that.

It's also less efficient as you have to read both the 
thread-local boolean and the __gshared instance. Since the 
thread-local boolean is likely going to use a word anyway you may 
as well store the instance in there instead.

Single reads are NOT atomic. On x86 word-aligned reads *happen* 
to be atomic, and even that is not guaranteed on other 
architectures. The main advantage of the low-lock singleton idea 
is that it is completely independent of architecture (there are 
more efficient ways if the architecture is known).

With respect to "hasInstance", what is a possible use-case where 
synchronisation is not required?
May 19 2013
next sibling parent "Idan Arye" <GenericNPC gmail.com> writes:
On Sunday, 19 May 2013 at 20:35:06 UTC, Diggory wrote:

 There is no point in saving a thread local reference to the 
 global instance. The `__gshared` instance is never changed 
 once initialized, so if we saved a thread local reference, it 
 would *always* be either null or the same as the `__gshared` 
 one - which means that if the local reference is not null, 
 there is no difference between returning the local and the 
 global references.

 `hasInstance` does not need no synchronization - it would just 
 slow it down. Synchronization is redundant in readonly and 
 writeonly scenarios - and this is a readonly scenario. A 
 single read is atomic with or without a synchronization.

 At any rate, using my implementation was broekn - I forgot to 
 set the thread local boolean instantiation indicator to 
 true(which would mean there will always be a lock!). I fixed 
 it. Thanks for pointing that out!


 With regard to using a boolean instead of storing the instance 
 thread locally - you're still reading from a mutable __gshared 
 variable with no synchronisation on the reader's side, and that 
 is always a bug. It may work in most cases but due to 
 instruction reordering and differences between architectures 
 there's no guarantee of that.

 It's also less efficient as you have to read both the 
 thread-local boolean and the __gshared instance. Since the 
 thread-local boolean is likely going to use a word anyway you 
 may as well store the instance in there instead.

 Single reads are NOT atomic. On x86 word-aligned reads *happen* 
 to be atomic, and even that is not guaranteed on other 
 architectures. The main advantage of the low-lock singleton 
 idea is that it is completely independent of architecture 
 (there are more efficient ways if the architecture is known).

 With respect to "hasInstance", what is a possible use-case 
 where synchronisation is not required?


Reading an aligned word-sized value should be atomic, pointers 
are word-sized, and compilers usually align variables - so I do 
believe it's safe to treat reads as atomic.

And even if they weren't - there should be no problem regarding 
`hasInstance`, because it returns a boolean value - true or 
false. That means there are for options:

1) `hasInstance` returns true when the instance was already 
initialized.
2) `hasInstance` returns false when the instance was not yet 
initialized.
3) `hasInstance` returns false while the instance is being 
initialized in another thread.
4) `hasInstance` returns true while the instance is being 
initialized in another thread.

Obviously we have no problem with 1 and 2, but 3 and 4 seem 
problematic. I claim they are not!

Let's take a look at 3. `hasInstance` returns false even thought 
an instance is currently being initialized. But if `hasInstace` 
was invoked a nanosecond earlier it would have also returned 
false, and this time it would be correct. In both 
cases(nanosecond earlier and nanosecond later), by the time 
thread that called `hasInstance` has reach the next command the 
singleton will be instantiated and the answer of `hasInstance` 
will be outdated. So - scenario 3 does not introduce any new bugs.

Scenario 4, however, seems to be a bit more problematic - when 
`hasInstance` returns true even though the singleton has not been 
fully instantiated. What does that mean? well, the only way a 
read or a write can be non-atomic is if the memory is 
read/written in parts:
  - Maybe the read split - maybe `hasInstance` read half the 
variable, then the other thread written, then `hasInstance` read 
the other half.
  - Maybe the write split - maybe the other thread wrote half the 
variable, then `hasInstance` read it, then the other thread wrote 
the other half.
  - And maybe both of them split.
At any rate, no matter was was split, the result is the same - 
`hasInstance` will return true because what it read from the 
global reference is not null(even though it's not a full 
reference). This seems bad - `hasInstance` tells us that the 
singleton is already instantiated even though the other thread 
didn't finish writing the reference before it was invoked!

Well, I say it doesn't matter. By the time `hasInstance` will 
return it's value and the code can act upon it, the thread that 
finished the singleton will surely have finished writing that 
single global reference!

And even if it didn't - we can rest assured that we can rely on 
`hasInstance`'s answer, because it tell us that the singleton can 
no longer be initialized, and that by the time we need to access 
it, it will already be initialized. And even if by then the other 
thread would still not finished writing the reference - maybe 
because the thread that called `hasInstance` is being run on a 
3.5GHz core and the thread that initiates the singleton instance 
is being run by a monkey with an abacus - once you try to 
actually access that instance you will enter that synchronization 
block that will hold you until the monkey finish writing the 
reference and it is ready to use.



Beside `hasInstance`, there is two other place where I am reading 
the `__gshared` reference without synchronization - both at 
`instance()`. One is a null check in case singleton does not 
support implicit instantiation - and now that I look at it, maybe 
I should put a synchronization there(though I still don't think 
it's needed). The other one is the return statement - but by the 
time we reach it, we know that the instantiation was already 
completed, and we know that the the reference will never change 
again until the end of the process - in other words, we know 
there will not be any more writes to this `__gshared` reference, 
and that means we can read it safely without synchronizations 
just we can safely read immutable values without synchronization.
May 19 2013
prev sibling parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Sunday, 19 May 2013 at 20:35:06 UTC, Diggory wrote:

 It's also less efficient as you have to read both the 
 thread-local boolean and the __gshared instance. Since the 
 thread-local boolean is likely going to use a word anyway you 
 may as well store the instance in there instead.


This is a good point. I've changed the local indicator to a local 
reference.

I've also added the local check under `instance()`. Until now, I 
relayed on `singleton_tryInitInstance()` to do that check, but I 
figured out that it's better to do an extra local check on first 
call(in each thread) than to do an extra method call for every 
call afterward. Also, when that check is inside another method, 
the optimization you suggested won't work(unless the compiler 
decides to inline it)
May 19 2013
parent reply "Diggory" <diggsey googlemail.com> writes:
In your logic you're assuming that the order of operations is 
maintained - without the correct memory barriers or 
synchronisation both the compiler and CPU are free to completely 
reorder any operations you do. That's why it's always a bug to 
access mutable shared data without synchronisation - any read not 
using some form of synchronisation could give you back any value 
that the memory is ever set to during the course of the program.
May 19 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 20 May 2013 at 05:39:42 UTC, Diggory wrote:

 In your logic you're assuming that the order of operations is 
 maintained - without the correct memory barriers or 
 synchronisation both the compiler and CPU are free to 
 completely reorder any operations you do. That's why it's 
 always a bug to access mutable shared data without 
 synchronisation - any read not using some form of 
 synchronisation could give you back any value that the memory 
 is ever set to during the course of the program.


It can't be THAT chaotic. Neither the compiler nor the CPU will 
perform a check on a value and then go back in time to fetch an 
old version of that value and return it. This kind of behavior 
would break non-threaded code.
May 19 2013
parent reply "Diggory" <diggsey googlemail.com> writes:
On Monday, 20 May 2013 at 06:53:34 UTC, Idan Arye wrote:

 On Monday, 20 May 2013 at 05:39:42 UTC, Diggory wrote:

 In your logic you're assuming that the order of operations is 
 maintained - without the correct memory barriers or 
 synchronisation both the compiler and CPU are free to 
 completely reorder any operations you do. That's why it's 
 always a bug to access mutable shared data without 
 synchronisation - any read not using some form of 
 synchronisation could give you back any value that the memory 
 is ever set to during the course of the program.


 It can't be THAT chaotic. Neither the compiler nor the CPU will 
 perform a check on a value and then go back in time to fetch an 
 old version of that value and return it. This kind of behavior 
 would break non-threaded code.


Of course it's possible, for example the code may produce the 
expected result if some invariant holds which does in fact hold 
if there was a single thread running, but with multiple threads 
the invariant is broken. Or more simply - the fact remains that 
you are writing on one thread (correctly using synchronisation) 
and reading from another (not using synchronisation) and 
synchronisation is required on both the read and the write. The 
compiler/CPU is then free to reorder the reads under the 
assumption that the value won't change, and this assumption is 
clearly wrong.

Basically most of your argument is just hoping that it will 
behave the way you want, but without having any real guarantees, 
and that's not the way to write thread-safe code, especially if 
it's going to be part of the standard library.
May 20 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 20 May 2013 at 11:19:44 UTC, Diggory wrote:

 Of course it's possible, for example the code may produce the 
 expected result if some invariant holds which does in fact hold 
 if there was a single thread running, but with multiple threads 
 the invariant is broken. Or more simply - the fact remains that 
 you are writing on one thread (correctly using synchronisation) 
 and reading from another (not using synchronisation) and 
 synchronisation is required on both the read and the write. The 
 compiler/CPU is then free to reorder the reads under the 
 assumption that the value won't change, and this assumption is 
 clearly wrong.


I do not assume that the compiler or the CPU will not change the 
order of reads in the unsynchronized thread - I showed that the 
result of `hasInstance()` is not affected by such reordering! 
`hasInstance()` has a single read to __gshared memory, and the 
only thing that can effect the result of that read is a write to 
that memory, which is done *once* in the synchronized thread. 
That means I should only care when the read in `hasInstance()` 
happens related to that write.

I have shown that whether the read happens before the write, or 
the write happens before the read, or they happen at the same 
time, or the write is split and the read is done between the two 
parts of the write, or the other way around, or if both the read 
and write are split and their parts are performed alternately - 
no matter what, `hasInstance()` yields a result I can rely on.

Since `hasInstance()` produces reliable results even if it gets 
mixed in the timeframe of the instantiation in another thread - I 
see no reason to do a costly synchronization to prevent the 
mixing.



I have tried to form a similar proof for the static branch in 
`instance()` that handles the no-default-constructor case, and 
realized that this one does need synchronization, because the 
compiler might decide to set the reference before it runs the 
initialization of the object. Even though the result it will 
return will be the correct reference to the instance, the 
instance object itself might not be ready.

So, I'm making that part synchronized, simply to force 
`instance()` to wait until the instance object has finished it's 
instantiation.
May 20 2013
parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
20-May-2013 19:28, Idan Arye пишет:

 On Monday, 20 May 2013 at 11:19:44 UTC, Diggory wrote:

 Of course it's possible, for example the code may produce the expected
 result if some invariant holds which does in fact hold if there was a
 single thread running, but with multiple threads the invariant is
 broken. Or more simply - the fact remains that you are writing on one
 thread (correctly using synchronisation) and reading from another (not
 using synchronisation) and synchronisation is required on both the
 read and the write. The compiler/CPU is then free to reorder the reads
 under the assumption that the value won't change, and this assumption
 is clearly wrong.


 I do not assume that the compiler or the CPU will not change the order
 of reads in the unsynchronized thread - I showed that the result of
 `hasInstance()` is not affected by such reordering! `hasInstance()` has
 a single read to __gshared memory, and the only thing that can effect
 the result of that read is a write to that memory, which is done *once*
 in the synchronized thread. That means I should only care when the read
 in `hasInstance()` happens related to that write.


Long story short - re-read the discussion in the Low-lock thread again:
http://forum.dlang.org/thread/pelhvaxwjzhehdjtpsav forum.dlang.org


-- 
Dmitry Olshansky
May 20 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 20 May 2013 at 16:40:27 UTC, Dmitry Olshansky wrote:

 Long story short - re-read the discussion in the Low-lock 
 thread again:
 http://forum.dlang.org/thread/pelhvaxwjzhehdjtpsav forum.dlang.org


To sum up the discussion, there are three problems with 
unsynchronized access to the __gshared instance reference:

1) Unbaked object - the writer might write the __gshared 
reference before it finished the construction of the actual 
object.

2) Non-atomic read/write - this could result in a bad reference 
where the reader get some bytes from the old reference and some 
bytes from the new reference.

3) Unsynchronized cache - reader reads the reference, but the 
part of it's cache that is mapped to the memory containing the 
object instance itself is out of date and has not received the 
object yet.

All these problems do not affect `hasInstance()` - which is the 
only part of my implementation that touches the __gshared 
reference without synchronization - simply because it does not 
touch the object and does not return the reference - it only 
checks if it's initialized:

1) It doesn't matter if the object is not ready, because when you 
want to actually access the object, you need to use `instance()` 
which has synchronization.

2) It does not matter if we get half a reference due to 
non-atomic read/write, because we only care if it's null or not. 
If the half reference we got is not null, that means the whole 
reference is not null and we have the correct answer. If the half 
reference we got is null - well, maybe the other half is not 
null, but the reference is only now being made not-null, so no 
harm is done it treating it as null for now(if we actually try to 
initialize it we will enter synchronization).

3) Since we don't try to access the object itself, we don't care 
that our local cache doesn't have it yet. Again - once we reach 
for the object itself, we will enter synchronization.
May 20 2013
parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
20-May-2013 22:14, Idan Arye пишет:

 On Monday, 20 May 2013 at 16:40:27 UTC, Dmitry Olshansky wrote:

 Long story short - re-read the discussion in the Low-lock thread again:
 http://forum.dlang.org/thread/pelhvaxwjzhehdjtpsav forum.dlang.org


 To sum up the discussion, there are three problems with unsynchronized
 access to the __gshared instance reference:

 1) Unbaked object - the writer might write the __gshared reference
 before it finished the construction of the actual object.

 2) Non-atomic read/write - this could result in a bad reference where
 the reader get some bytes from the old reference and some bytes from the
 new reference.

 3) Unsynchronized cache - reader reads the reference, but the part of
 it's cache that is mapped to the memory containing the object instance
 itself is out of date and has not received the object yet.

 All these problems do not affect `hasInstance()` - which is the only


2 & 3 do.


 part of my implementation that touches the __gshared reference without
 synchronization - simply because it does not touch the object and does
 not return the reference - it only checks if it's initialized:


It _reads_ the _shared_ reference without proper indication of this 
intent to the writers.


 1) It doesn't matter if the object is not ready, because when you want
 to actually access the object, you need to use `instance()` which has
 synchronization.


Then where you see hasInstance to fit the bill?


 2) It does not matter if we get half a reference due to non-atomic
 read/write, because we only care if it's null or not. If the half
 reference we got is not null, that means the whole reference is not null
 and we have the correct answer.


Or you may never get the reference updated until the cache got flushed. 
It's generally not defined when you will see the update until then (or 
some such hardware event). The word is *eventually*.


 If the half reference we got is null -
 well, maybe the other half is not null, but the reference is only now
 being made not-null, so no harm is done it treating it as null for
 now(if we actually try to initialize it we will enter synchronization).


So it doesn't matter if hasInstance is fulfilling it's questionable 
contract properly only sometimes.


 3) Since we don't try to access the object itself, we don't care that
 our local cache doesn't have it yet. Again - once we reach for the
 object itself, we will enter synchronization.


The big question is how you imagine somebody would want to use this

The false case may stay this way for unknown amount of time for instance 
even after the initialization happened (on some architectures). At the 
very least make that read atomicLoad that will make the operation 
properly tied to the current view of memory.

Even if we assume it's atomic then the other big question is what is the 
use case.
I argue that hasInstance only welcomes poor designs like:

while(!hasInstance()){
	//busy wait  for somebody else to init it?
}
inst = instance();

or:
if(hasInstance()){
	//should be initialized then the call won't construct it
	... //dunno what advantage it gets
}
else{
	//might or might not initialize/block on call to instance()
	...//again dunno
}


I'd say:

If you need to poke under it to avoid initialization then you shouldn't 
be using lazy-init singleton in the first place.

If you need synchronization and coordination based on what the reference 
happens to be right now then there are tools far better fit for the job 
- mutexes, semaphore, condition vars etc.

The last but not least is the fact that LowLock returns TLS reference to 
a (__g)shared instance make me worry about how the users code now is 
full of hidden race conditions anyway. This applies to the pattern as 
presented not only your implementation of it.
So the singleton itself would need some synchronization... and for that 
it really should be marked shared. The alternative is to have a 
per-thread singleton without any locking.

-- 
Dmitry Olshansky
May 20 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 20 May 2013 at 19:15:34 UTC, Dmitry Olshansky wrote:

 20-May-2013 22:14, Idan Arye пишет:

 1) It doesn't matter if the object is not ready, because when 
 you want
 to actually access the object, you need to use `instance()` 
 which has
 synchronization.


 Then where you see hasInstance to fit the bill?


`hasInstance()` tells you if an instance is already initialized, 
without returning it and without risking in initializing it if it 
isn't already initialized.


 2) It does not matter if we get half a reference due to 
 non-atomic
 read/write, because we only care if it's null or not. If the 
 half
 reference we got is not null, that means the whole reference 
 is not null
 and we have the correct answer.


 Or you may never get the reference updated until the cache got 
 flushed. It's generally not defined when you will see the 
 update until then (or some such hardware event). The word is 
 *eventually*.


When you call `instance()` to fetch the reference(opposed to 
`hasInstance()`, which only tells you if it's null), the thread 
will enter the synchronization block inside `instance()` which 
will make sure the cache is refreshed before it begins.


 If the half reference we got is null -
 well, maybe the other half is not null, but the reference is 
 only now
 being made not-null, so no harm is done it treating it as null 
 for
 now(if we actually try to initialize it we will enter 
 synchronization).


 So it doesn't matter if hasInstance is fulfilling it's 
 questionable contract properly only sometimes.


If `hasInstance()` returns `true`, you can assume that there is 
an instance for you to access, because even if the instance is 
not ready yet, some other thread has entered the synchronization 
block and the user can't get the instance until that thread 
finishes the instantiation - so from the thread's point of view, 
whenever it'll call `instance()` it'll get a ready instance that 
was not created beacause of it.

If `hasInstance()` returns `false` then it's not a guarantee that 
the singleton has not been instantiated, but even if 
`hasInstance()` was synchronized you wouldn't get that guarantee, 
because it is always possible that the singleton is instantiated 
after the synchronization in `hasInstance()` but before you get 
to act upon it's result.

The only way to get a `false` with that guarantee is to make the 
synchronization from outside `hasInstance()`. So, not using 
synchronization inside it does not break any contract an 
internally synchronized `hasInstance()` could promise.


 3) Since we don't try to access the object itself, we don't 
 care that
 our local cache doesn't have it yet. Again - once we reach for 
 the
 object itself, we will enter synchronization.


 The big question is how you imagine somebody would want to use 
 this

 The false case may stay this way for unknown amount of time for 
 instance even after the initialization happened (on some 
 architectures). At the very least make that read atomicLoad 
 that will make the operation properly tied to the current view 
 of memory.


`atomicLoad` requires a `shared` reference. Using it will force 
me to turn the singleton into a shared singleton. Maybe I should 
add a shared version(I'll return to that at the end of this 
response), but I still want to keep the __gshared version.


 Even if we assume it's atomic then the other big question is 
 what is the use case.
 I argue that hasInstance only welcomes poor designs like:

 while(!hasInstance()){
 	//busy wait  for somebody else to init it?
 }
 inst = instance();

 or:
 if(hasInstance()){
 	//should be initialized then the call won't construct it
 	... //dunno what advantage it gets
 }
 else{
 	//might or might not initialize/block on call to instance()
 	...//again dunno
 }


 I'd say:

 If you need to poke under it to avoid initialization then you 
 shouldn't be using lazy-init singleton in the first place.

 If you need synchronization and coordination based on what the 
 reference happens to be right now then there are tools far 
 better fit for the job - mutexes, semaphore, condition vars etc.


First and foremost - this is library library code, so it should 
expose as much interface as possible without exposing or breaking 
the implementation.

Personally, I think `hasInstance()` would be mainly used in the 
second use case you suggested. It would be useful if you have a 
long running loop(for example - a game loop) that needs to 
interact with the singleton instance if it exists, but can't or 
shouldn't instantiate it.

As for the first use case, you are right - it is a bad design to 
busy-wait for a singleton to be instantiated somewhere else. I 
should probably add another method that waits for the instance 
using a condition.


 The last but not least is the fact that LowLock returns TLS 
 reference to a (__g)shared instance make me worry about how the 
 users code now is full of hidden race conditions anyway. This 
 applies to the pattern as presented not only your 
 implementation of it.
 So the singleton itself would need some synchronization... and 
 for that it really should be marked shared. The alternative is 
 to have a per-thread singleton without any locking.


There is also a thread local version called 
`ThreadLocalSingleton`. If I made a shared version, would that 
solve those possible hidden race conditions? Is there a point in 
using the TLS Low Lock method for shared singletons?
May 20 2013
next sibling parent reply "Diggory" <diggsey googlemail.com> writes:
On Monday, 20 May 2013 at 22:02:57 UTC, Idan Arye wrote:

 On Monday, 20 May 2013 at 19:15:34 UTC, Dmitry Olshansky wrote:

 20-May-2013 22:14, Idan Arye пишет:

 1) It doesn't matter if the object is not ready, because when 
 you want
 to actually access the object, you need to use `instance()` 
 which has
 synchronization.


 Then where you see hasInstance to fit the bill?


 `hasInstance()` tells you if an instance is already 
 initialized, without returning it and without risking in 
 initializing it if it isn't already initialized.


Saying what it does is not a use-case.



 If `hasInstance()` returns `true`, you can assume that there is 
 an instance for you to access, because even if the instance is 
 not ready yet, some other thread has entered the 
 synchronization block and the user can't get the instance until 
 that thread finishes the instantiation - so from the thread's 
 point of view, whenever it'll call `instance()` it'll get a 
 ready instance that was not created beacause of it.


It can return true even if the instance has not been initialised 
yet or false if it has because there's no synchronisation on the 
read. Given that in either situation it can return either true or 
false, there's no use for it.


 First and foremost - this is library library code, so it should 
 expose as much interface as possible without exposing or 
 breaking the implementation.


Only if the interface is useful and not likely to promote bad 
code... It's better to give someone multiple tools, each well 
suited for its purpose, than one tool that tries to do everything 
but is not well suited to anything.


 Personally, I think `hasInstance()` would be mainly used in the 
 second use case you suggested. It would be useful if you have a 
 long running loop(for example - a game loop) that needs to 
 interact with the singleton instance if it exists, but can't or 
 shouldn't instantiate it.


In the second example you may as well do:
if (getRandomBool()) {
     ...
} else {
     ...
}

It has the exact same guarantees as your code.

So what's the point of 'hasInstance'?
May 20 2013
parent "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 20 May 2013 at 23:18:59 UTC, Diggory wrote:

 On Monday, 20 May 2013 at 22:02:57 UTC, Idan Arye wrote:

 If `hasInstance()` returns `true`, you can assume that there 
 is an instance for you to access, because even if the instance 
 is not ready yet, some other thread has entered the 
 synchronization block and the user can't get the instance 
 until that thread finishes the instantiation - so from the 
 thread's point of view, whenever it'll call `instance()` it'll 
 get a ready instance that was not created beacause of it.


 It can return true even if the instance has not been 
 initialised yet or false if it has because there's no 
 synchronisation on the read. Given that in either situation it 
 can return either true or false, there's no use for it.


If it returns `true` that means that either the instance is 
already initialized, or that it is currently being initialized in 
another thread. For all threads but the one that initializes the 
singleton, there is no difference between these two states:

1) a call to `instance()` will *not* create a new instance, and 
will return the *ready* instance.
2) a call to `singleton_tryInitInstance()` will *not* invoke the 
lazy initializer argument and will return `false`.
3) a call to `singleton_initInstance()` will *not* invoke the 
lazy initializer argument and will throw a `SingletonException`.

and ofcourse:

4) a call to `hasInstance()` will return `true`.

The only thing that will behave differently is a direct access to 
`__singleton_instance` - but this is something users of this 
library shouldn't be doing anyways!


 First and foremost - this is library library code, so it 
 should expose as much interface as possible without exposing 
 or breaking the implementation.


 Only if the interface is useful and not likely to promote bad 
 code... It's better to give someone multiple tools, each well 
 suited for its purpose, than one tool that tries to do 
 everything but is not well suited to anything.


Programmers are lazy - they won't check if a singleton has been 
instantiated just for fun, not when the singleton handles 
instantiation automatically, or the method for manual 
instantiation does that check for you.

Bad usage of `hasInstance()`(beside busy waiting that Dmitry 
Olshansky mentioned - I'm going to provide a condition waiting 
function for that) involves more effort and gives nothing in 
return - therefore, people won't use it.


 Personally, I think `hasInstance()` would be mainly used in 
 the second use case you suggested. It would be useful if you 
 have a long running loop(for example - a game loop) that needs 
 to interact with the singleton instance if it exists, but 
 can't or shouldn't instantiate it.


 In the second example you may as well do:
 if (getRandomBool()) {
     ...
 } else {
     ...
 }

 It has the exact same guarantees as your code.

 So what's the point of 'hasInstance'?


     class Foo{
         mixin LowLockSingleton;

         public this(int x){
             ...
         }

         public void bar(){
             ...
         }
     }

     ......

     if(getRandomBool()){
         /*1*/ Foo.instance.bar();
     }

     if(Foo.hasInstance){
         /*2*/ Foo.instance.bar();
     }

/*1*/ has a chance to throw an `SingletonException` if `Foo` has 
not been instantiated yet.
/*2*/ will not throw(unless `bar()` throws), because if 
`Foo.hasInstance` returns true, that means that either:
1) Foo is already instantiated.
2) Foo is in the instantiation process, which means that 
`Foo.instance` will reach a synchronization block, wait until 
`Foo` is instantiated.
In both of those cases, the fact that `Foo.hasInstance` returned 
`true` guarantee that `Foo.instance` will return a reference to a 
ready-to-use instance.
May 20 2013
prev sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
21-May-2013 02:02, Idan Arye пишет:

 On Monday, 20 May 2013 at 19:15:34 UTC, Dmitry Olshansky wrote:



 If you need synchronization and coordination based on what the
 reference happens to be right now then there are tools far better fit
 for the job - mutexes, semaphore, condition vars etc.


 First and foremost - this is library library code, so it should expose
 as much interface as possible without exposing or breaking the
 implementation.


A-ha. That's it and it's totally wrong. Exposing as much interface as 
possible is a disaster. Libraries (esp standard) take great deal of 
deliberation in picking what to expose. Exposing less is a common theme 
in interfaces. "Doing more" is a common theme in wrappers, helpers and 
has a proverbial "kitchen sink" effect.


 Personally, I think `hasInstance()` would be mainly used in the second
 use case you suggested. It would be useful if you have a long running
 loop(for example - a game loop) that needs to interact with the
 singleton instance if it exists, but can't or shouldn't instantiate it.


Message passing, queues, condition variables. Singleton pattern assumes 
you have an object with unknown initialization order and who ever 
touches it first gets to create it.


 As for the first use case, you are right - it is a bad design to
 busy-wait for a singleton to be instantiated somewhere else. I should
 probably add another method that waits for the instance using a condition.


And your are falling into a trap I decidedly put in my example - this is 
a no use case for singletons. Want to wait on smth to happen? - use 
*separate* condition var, event pump etc. Want to check on some external 
state? - same answer.

Though you might provide separate class for waitable singleton that 
incorporates
condition variable. Could be useful sometimes(?) in case there is no 
other logic
to define order of initialization.

BTW Why not just make a template Singleton!T instead of mixin?


 The last but not least is the fact that LowLock returns TLS reference
 to a (__g)shared instance make me worry about how the users code now
 is full of hidden race conditions anyway. This applies to the pattern
 as presented not only your implementation of it.
 So the singleton itself would need some synchronization... and for
 that it really should be marked shared. The alternative is to have a
 per-thread singleton without any locking.


 There is also a thread local version called `ThreadLocalSingleton`. If I
 made a shared version, would that solve those possible hidden race
 conditions?


It would make people do something about it - shared allows only calling 
shared methods of a class and prohibits all basic operations on the 
fields. Points of race become fairly obvious - they need casts and lack 
of locks in the vicinity.

Every time I see __gshared I think "lazy, old and broken" and usually at 
least one of these is true.


 Is there a point in using the TLS Low Lock method for shared
 singletons?


Good question. Yes, it is as it will allow you to have lazy 
initialization from any thread on any multicores w/o always taking a 
lock. If you can't figure out how to make order of initialization 
deterministic (or who creates what and when) then these lazy-init 
singletons are good idea.

I personally despise singletons and the code patterns they introduce. 
IMHO lazy initialization (not to mistake with lazy loading/caching) is 
so rarely *required* that you may as well avoid it, and in D we have 
static this/shared static this.

-- 
Dmitry Olshansky
May 20 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Tuesday, 21 May 2013 at 06:44:02 UTC, Dmitry Olshansky wrote:

 A-ha. That's it and it's totally wrong. Exposing as much 
 interface as possible is a disaster. Libraries (esp standard) 
 take great deal of deliberation in picking what to expose. 
 Exposing less is a common theme in interfaces. "Doing more" is 
 a common theme in wrappers, helpers and has a proverbial 
 "kitchen sink" effect.


 ...


 Message passing, queues, condition variables. Singleton pattern 
 assumes you have an object with unknown initialization order 
 and who ever touches it first gets to create it.


 ...


 And your are falling into a trap I decidedly put in my example 
 - this is a no use case for singletons. Want to wait on smth to 
 happen? - use *separate* condition var, event pump etc. Want to 
 check on some external state? - same answer.


The user of a library usually knows what they want to do with the 
library much more than the designer of the library. I see no 
reason to force the user to hack around trying to get information 
that the library could easily provide them without breaking 
anything or exposing the implementation.


 Though you might provide separate class for waitable singleton 
 that incorporates
 condition variable. Could be useful sometimes(?) in case there 
 is no other logic
 to define order of initialization.

 BTW Why not just make a template Singleton!T instead of mixin?


Something like that is possible, but I believe it's place is in 
`std.idioms` but in `std.parallelism`, since it is very similar 
to `Task` - a structure that represents a calculation(in this 
case - the initialization) that will be performed sooner or 
later, and provides synchronization on the invocation, 
synchronized access to the result and information whether it 
happened already or not.


 It would make people do something about it - shared allows only 
 calling shared methods of a class and prohibits all basic 
 operations on the fields. Points of race become fairly obvious 
 - they need casts and lack of locks in the vicinity.

 Every time I see __gshared I think "lazy, old and broken" and 
 usually at least one of these is true.


D provides 3 types of global - `shared`, `__gshared` and 
`static`(which is only global in the same thread). Since a 
singleton is a lazy global, I'm gonna provide a implementation an 
implementation for each type of global D has and let the users 
choose, like they do with regular globals.

Though, it might be a good idea to rename it to 
`__GSharedSingleton`. On one hand, the double underline prefix 
represents an implementation detail. On the other hand, using 
just `GSharedSingleton` will be too similar to `SharedSingleton` 
and might confuse even more.


 Is there a point in using the TLS Low Lock method for shared
 singletons?


 Good question. Yes, it is as it will allow you to have lazy 
 initialization from any thread on any multicores w/o always 
 taking a lock. If you can't figure out how to make order of 
 initialization deterministic (or who creates what and when) 
 then these lazy-init singletons are good idea.


Is it possible to save a thread local copy of the lock though? I 
can't use `static`, because it'll have to be `static shared` to 
keep the `shared`ness of the instance, which will not be thread 
local.

Ofcourse, using a boolean is always possible, it'll just be 
slower.


 I personally despise singletons and the code patterns they 
 introduce. IMHO lazy initialization (not to mistake with lazy 
 loading/caching) is so rarely *required* that you may as well 
 avoid it, and in D we have static this/shared static this.


Even if you think singletons are bad, you have to agree that some 
ways to implement them are far worse than others. My library 
implementation would be better than most project local 
implementations because I have put in it much more time and 
effort than the what's dedicated to such a common pattern in a 
large project, and because it is being peer reviewed here.
May 21 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
At any rate, I am forced to admit I made a mistake about 
`hasIntance()` not needing synchronization. I neglected the 
possibility that the constructor(or anything else used for 
initialization) can throw!

The compiler might decide that it's better to write the global 
reference first, and if the initializer throws just set it back 
to null. If that is the case, `hasInstance()` could see that the 
global reference is not null and return true, and then the 
initializer will throw and the initializing thread will set the 
global reference to null again.

So yea, I'm gonna have to synchronize it too...
May 21 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Tuesday, 21 May 2013 at 14:58:16 UTC, Idan Arye wrote:

 At any rate, I am forced to admit I made a mistake about 
 `hasIntance()` not needing synchronization. I neglected the 
 possibility that the constructor(or anything else used for 
 initialization) can throw!

 The compiler might decide that it's better to write the global 
 reference first, and if the initializer throws just set it back 
 to null. If that is the case, `hasInstance()` could see that 
 the global reference is not null and return true, and then the 
 initializer will throw and the initializing thread will set the 
 global reference to null again.

 So yea, I'm gonna have to synchronize it too...


OK, it is done.

Next step - write the introduction. And then add the shared 
version of the singleton.
May 21 2013
parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Tuesday, 21 May 2013 at 17:40:02 UTC, Idan Arye wrote:

 On Tuesday, 21 May 2013 at 14:58:16 UTC, Idan Arye wrote:

 At any rate, I am forced to admit I made a mistake about 
 `hasIntance()` not needing synchronization. I neglected the 
 possibility that the constructor(or anything else used for 
 initialization) can throw!

 The compiler might decide that it's better to write the global 
 reference first, and if the initializer throws just set it 
 back to null. If that is the case, `hasInstance()` could see 
 that the global reference is not null and return true, and 
 then the initializer will throw and the initializing thread 
 will set the global reference to null again.

 So yea, I'm gonna have to synchronize it too...


 OK, it is done.

 Next step - write the introduction. And then add the shared 
 version of the singleton.


OK, I've written and pushed the documentation. I'll try to find 
somewhere to host the generated HTML.

I've also added `SharedSingleton` and renamed `LowLockSingleton` 
to `__GSharedSingleton` for consistency. I *did not* rename 
`ThreadLocalSingleton` to `StaticSingleton` - this will be too 
confusing("aren't all singletons static?").
May 22 2013
parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
23-May-2013 01:23, Idan Arye пишет:

 On Tuesday, 21 May 2013 at 17:40:02 UTC, Idan Arye wrote:

 On Tuesday, 21 May 2013 at 14:58:16 UTC, Idan Arye wrote:

 At any rate, I am forced to admit I made a mistake about
 `hasIntance()` not needing synchronization. I neglected the
 possibility that the constructor(or anything else used for
 initialization) can throw!

 The compiler might decide that it's better to write the global
 reference first, and if the initializer throws just set it back to
 null. If that is the case, `hasInstance()` could see that the global
 reference is not null and return true, and then the initializer will
 throw and the initializing thread will set the global reference to
 null again.

 So yea, I'm gonna have to synchronize it too...


 OK, it is done.

 Next step - write the introduction. And then add the shared version of
 the singleton.


 OK, I've written and pushed the documentation. I'll try to find
 somewhere to host the generated HTML.


I've found github pages to be just fine.


 I've also added `SharedSingleton` and renamed `LowLockSingleton` to
 `__GSharedSingleton` for consistency. I *did not* rename
 `ThreadLocalSingleton` to `StaticSingleton` - this will be too
 confusing("aren't all singletons static?").


Turns out that doing shared classes (and singletons conversely) has one 
big of disadvantage: you can't have TLS reference to shared class 
instance (ditto with mutable reference to const object).

It's a well-known problem of the way OOP is done in D - object and 
reference to it are tightly coupled and qualifier applies transitively 
to both.

There was a pull that allowed to separate qualifier of instance from 
reference (handle) looking like this:

ref const(Object) refToConst;

ref Object mutableTlsRef;
Object mutableTlsRef; //same as above

ref const Object constRefToConst;
const Object constRefToConst; //ditto

The fact that we don't have it is part of the reason I don't like doing 
OOP in D at all.

-- 
Dmitry Olshansky
May 24 2013
next sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Friday, May 24, 2013 17:42:59 Dmitry Olshansky wrote:

 There was a pull that allowed to separate qualifier of instance from
 reference (handle) looking like this:
 
 ref const(Object) refToConst;
 
 ref Object mutableTlsRef;
 Object mutableTlsRef; //same as above
 
 ref const Object constRefToConst;
 const Object constRefToConst; //ditto
 
 The fact that we don't have it is part of the reason I don't like doing
 OOP in D at all.


Lacking a proper language solution, we could create something similar to 
Rebindable but for shared.

- Jonathan M Davis
May 24 2013
parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
24-May-2013 22:13, Jonathan M Davis пишет:

 On Friday, May 24, 2013 17:42:59 Dmitry Olshansky wrote:

 There was a pull that allowed to separate qualifier of instance from
 reference (handle) looking like this:

 ref const(Object) refToConst;

 ref Object mutableTlsRef;
 Object mutableTlsRef; //same as above

 ref const Object constRefToConst;
 const Object constRefToConst; //ditto

 The fact that we don't have it is part of the reason I don't like doing
 OOP in D at all.


 Lacking a proper language solution, we could create something similar to
 Rebindable but for shared.


Then in my vision built-in OOP has failed if we need at least 2 wrapper 
types implemented as structs on top of built-in refs. Then recall (to be 
implemented) RefCounted!ClassType and we have yet another library land 
solution to make OOP types with ref-counting.

On occasion I've been enumerating the amount of special casing class 
references require and truth be told I fail to see the benefit of having 
them as built-in outweigh the cost. With multiple alias this I could 
have implemented the OOP in library just fine as Ref!T struct. Then
Ref!(const(T)) is ConstRef!T
Ref!(shared(T)) is SharedRef!T

And now UDAs can serve for override/virtual, inheritance etc.

 - Jonathan M Davis



-- 
Dmitry Olshansky
May 24 2013
parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Saturday, May 25, 2013 00:08:37 Dmitry Olshansky wrote:

 24-May-2013 22:13, Jonathan M Davis пишет:

 Lacking a proper language solution, we could create something similar to
 Rebindable but for shared.


 
 Then in my vision built-in OOP has failed if we need at least 2 wrapper
 types implemented as structs on top of built-in refs. Then recall (to be
 implemented) RefCounted!ClassType and we have yet another library land
 solution to make OOP types with ref-counting.
 
 On occasion I've been enumerating the amount of special casing class
 references require and truth be told I fail to see the benefit of having
 them as built-in outweigh the cost. With multiple alias this I could
 have implemented the OOP in library just fine as Ref!T struct. Then
 Ref!(const(T)) is ConstRef!T
 Ref!(shared(T)) is SharedRef!T
 
 And now UDAs can serve for override/virtual, inheritance etc.


Overall, I think that OOP in D works fine, but we have some definite issues
with 
const due to how Object is currently set up, and the fact that the type system 
can't distinguish between a reference to a class object and the class object 
itself is definitely annoying, and I have no idea why it works that way (though 
my guess would be that it's simply the side effect of making it so that they 
have to live on the heap). We would definitely be better off if we could fix 
that. I do think that the remaining issues with OOP in D are quite fixable 
though. It just takes time and effort, and doing stuff like removing 
unnecessary functions from Object are probably blocked by the AA mess. So, we 
definitely have problems, but they're not insurmountable.

On the other hand, with regards to this particular problem, as Walter inquires 
in the bug report for not being able to have local references to shared 
objects, I have no idea what the use case for such would even be.

- Jonathan M Davis
May 24 2013
prev sibling parent reply "Idan Arye" <GenericNPC gmail.com> writes:
On Friday, 24 May 2013 at 13:43:02 UTC, Dmitry Olshansky wrote:

 I've found github pages to be just fine.


Then GitHub pages it is - 
http://someboddy.github.io/phobos/ddocs/for-idioms/idioms.html

I've also added this link to the pull request and to the review 
queue.


 Turns out that doing shared classes (and singletons conversely) 
 has one big of disadvantage: you can't have TLS reference to 
 shared class instance (ditto with mutable reference to const 
 object).

 It's a well-known problem of the way OOP is done in D - object 
 and reference to it are tightly coupled and qualifier applies 
 transitively to both.

 There was a pull that allowed to separate qualifier of instance 
 from reference (handle) looking like this:

 ref const(Object) refToConst;

 ref Object mutableTlsRef;
 Object mutableTlsRef; //same as above

 ref const Object constRefToConst;
 const Object constRefToConst; //ditto

 The fact that we don't have it is part of the reason I don't 
 like doing OOP in D at all.


Like Jonathan M Davis said, it could easily be added as a class. 
This is another example to D's flexibility - the ability to 
implement such things without changing the compiler. But still - 
a thing like this should be part of the language itself.

Anyways, since the implementation is so easy and short I could 
tweet it, I see no reason why not to make a pull request. It's 
gonna be in `std.typecons`, not in `std.idioms`, so I'll wait 
with using it in `std.idioms` until it gets pulled - or gets 
rejected in favor of a better(dmd) solution.
May 24 2013
parent "Idan Arye" <GenericNPC gmail.com> writes:
And here is the Phobos solution:

http://d.puremagic.com/issues/show_bug.cgi?id=10165
https://github.com/D-Programming-Language/phobos/pull/1302
May 24 2013
prev sibling parent reply "Jesse Phillips" <Jessekphillips+D gmail.com> writes:
On Saturday, 18 May 2013 at 16:58:19 UTC, Idan Arye wrote:

 OK, I implemented everything and made a pull request: 
 https://github.com/D-Programming-Language/phobos/pull/1294


I've added this to the review queue.

The module is in need of documentation (publishing not required) 
before it can be formally reviewed.
May 20 2013
parent reply "Jesse Phillips" <Jessekphillips+D gmail.com> writes:
On Monday, 20 May 2013 at 21:01:36 UTC, Jesse Phillips wrote:

 On Saturday, 18 May 2013 at 16:58:19 UTC, Idan Arye wrote:

 OK, I implemented everything and made a pull request: 
 https://github.com/D-Programming-Language/phobos/pull/1294


 I've added this to the review queue.

 The module is in need of documentation (publishing not 
 required) before it can be formally reviewed.


http://wiki.dlang.org/Review_Queue

Should have been more specific with this. The documentation looks 
to be sparse with few examples and little introduction. I'll have 
to do more examining of the specific functions/classes. Due to 
the nature of the module I may need to reconsider how much can 
actually go into an introduction.
May 20 2013
parent "Idan Arye" <GenericNPC gmail.com> writes:
On Monday, 20 May 2013 at 21:08:36 UTC, Jesse Phillips wrote:

 On Monday, 20 May 2013 at 21:01:36 UTC, Jesse Phillips wrote:

 On Saturday, 18 May 2013 at 16:58:19 UTC, Idan Arye wrote:

 OK, I implemented everything and made a pull request: 
 https://github.com/D-Programming-Language/phobos/pull/1294


 I've added this to the review queue.

 The module is in need of documentation (publishing not 
 required) before it can be formally reviewed.


 http://wiki.dlang.org/Review_Queue

 Should have been more specific with this. The documentation 
 looks to be sparse with few examples and little introduction. 
 I'll have to do more examining of the specific 
 functions/classes. Due to the nature of the module I may need 
 to reconsider how much can actually go into an introduction.


Well, obviously an example introduction like in `std.concurrency` 
or `std.variant` will not fit - since `std.idioms` is not 
centered around a single feature - but a table organized by 
idioms will be in order, and ofcourse - a short general 
description.

I'll get to that as soon as I can.
May 20 2013