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 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 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
prev sibling next sibling parent "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
prev sibling next sibling 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 "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
prev sibling 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 next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling 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 next 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 next 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
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 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 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 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
parent 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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling 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 next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling 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
prev sibling next sibling parent "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
prev sibling next sibling 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 next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling parent "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
prev sibling next sibling 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 "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