digitalmars.D - I want to add a Phobos module with template mixins for common idioms.
- Idan Arye (98/98) May 06 2013 Template mixins can be used to implement some common programming
- Diggory (12/12) May 06 2013 It's a nice idea but personally I don't like the syntax much, for
- Idan Arye (16/28) May 06 2013 If D supported Java's annotation metaprogramming I could have
- Idan Arye (4/6) May 06 2013 You know what? I take that back. I can probably pull this off
- Steven Schveighoffer (10/28) May 06 2013 D supports @Property syntax (though I would suggest a different
- Idan Arye (44/77) May 06 2013 I don't like this idea of marking the class with a `mixin`
- Jacob Carlborg (4/16) May 06 2013 Please no strings, it's horrible.
- Andrei Alexandrescu (7/11) May 06 2013 Nice idea, but at the first level of detail (without looking into it)
- Robert Rouse (2/16) May 06 2013 Since they are idioms, why not std.idioms or std.idiomatic?
- Andrei Alexandrescu (3/18) May 06 2013 std.patterns is the sweet spot.
- Idan Arye (4/31) May 06 2013 I like `std.idioms`. `std.patterns` can be ambiguous, since it
- Dicebot (9/9) May 07 2013 I really think that UDA annotations + single mixin like this:
- Idan Arye (76/85) May 07 2013 I think any method will scale(except for insane methods like
- Idan Arye (2/2) May 18 2013 OK, I implemented everything and made a pull request:
- Diggory (15/17) May 19 2013 Nice, but the singleton implementation seems somewhat
- Idan Arye (15/33) May 19 2013 There is no point in saving a thread local reference to the
- Diggory (17/32) May 19 2013 With regard to using a boolean instead of storing the instance
- Idan Arye (70/105) May 19 2013 Reading an aligned word-sized value should be atomic, pointers
- Idan Arye (10/14) May 19 2013 This is a good point. I've changed the local indicator to a local
- Diggory (7/7) May 19 2013 In your logic you're assuming that the order of operations is
- Idan Arye (5/13) May 19 2013 It can't be THAT chaotic. Neither the compiler nor the CPU will
- Diggory (15/28) May 20 2013 Of course it's possible, for example the code may produce the
- Idan Arye (29/39) May 20 2013 I do not assume that the compiler or the CPU will not change the
- Dmitry Olshansky (5/22) May 20 2013 Long story short - re-read the discussion in the Low-lock thread again:
- Idan Arye (32/35) May 20 2013 To sum up the discussion, there are three problems with
- Dmitry Olshansky (46/77) May 20 2013 It _reads_ the _shared_ reference without proper indication of this
- Idan Arye (45/114) May 20 2013 `hasInstance()` tells you if an instance is already initialized,
- Diggory (18/45) May 20 2013 It can return true even if the instance has not been initialised
- Idan Arye (51/83) May 20 2013 If it returns `true` that means that either the instance is
- Dmitry Olshansky (36/62) May 20 2013 A-ha. That's it and it's totally wrong. Exposing as much interface as
- Idan Arye (35/73) May 21 2013 The user of a library usually knows what they want to do with the
- Idan Arye (11/11) May 21 2013 At any rate, I am forced to admit I made a mistake about
- Idan Arye (4/15) May 21 2013 OK, it is done.
- Idan Arye (7/24) May 22 2013 OK, I've written and pushed the documentation. I'll try to find
- Dmitry Olshansky (19/45) May 24 2013 Turns out that doing shared classes (and singletons conversely) has one
- Jonathan M Davis (4/17) May 24 2013 Lacking a proper language solution, we could create something similar to...
- Dmitry Olshansky (14/31) May 24 2013 Then in my vision built-in OOP has failed if we need at least 2 wrapper
- Jonathan M Davis (15/32) May 24 2013 Overall, I think that OOP in D works fine, but we have some definite iss...
- Idan Arye (14/31) May 24 2013 Then GitHub pages it is -
- Idan Arye (3/3) May 24 2013 And here is the Phobos solution:
- Jesse Phillips (4/6) May 20 2013 I've added this to the review queue.
- Jesse Phillips (7/13) May 20 2013 http://wiki.dlang.org/Review_Queue
- Idan Arye (7/22) May 20 2013 Well, obviously an example introduction like in `std.concurrency`
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
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
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
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
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: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). -SteveIt'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...
May 06 2013
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: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...On Monday, 6 May 2013 at 19:47:33 UTC, Diggory wrote: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). -SteveIt'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...
May 06 2013
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
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
On Monday, 6 May 2013 at 19:57:27 UTC, Andrei Alexandrescu wrote:On 5/6/13 3:14 PM, Idan Arye wrote:Since they are idioms, why not std.idioms or std.idiomatic?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
On 5/6/13 4:03 PM, Robert Rouse wrote:On Monday, 6 May 2013 at 19:57:27 UTC, Andrei Alexandrescu wrote:std.patterns is the sweet spot. AndreiOn 5/6/13 3:14 PM, Idan Arye wrote:Since they are idioms, why not std.idioms or std.idiomatic?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
On Monday, 6 May 2013 at 20:10:59 UTC, Andrei Alexandrescu wrote:On 5/6/13 4:03 PM, Robert Rouse wrote: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.On Monday, 6 May 2013 at 19:57:27 UTC, Andrei Alexandrescu wrote:std.patterns is the sweet spot. AndreiOn 5/6/13 3:14 PM, Idan Arye wrote:Since they are idioms, why not std.idioms or std.idiomatic?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
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
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
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
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 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.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
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
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
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:That's a very nice syntax. It would be a good start for a mixin library.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
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:kickass technique, hope this get included soon, keep up the good work!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 13 2013
OK, I implemented everything and made a pull request: https://github.com/D-Programming-Language/phobos/pull/1294
May 18 2013
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/1294Nice, 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
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: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!OK, I implemented everything and made a pull request: https://github.com/D-Programming-Language/phobos/pull/1294Nice, 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
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
On Sunday, 19 May 2013 at 20:35:06 UTC, Diggory wrote: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.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
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
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
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
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: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.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 20 2013
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
20-May-2013 19:28, Idan Arye пишет:On Monday, 20 May 2013 at 11:19:44 UTC, Diggory wrote:Long story short - re-read the discussion in the Low-lock thread again: http://forum.dlang.org/thread/pelhvaxwjzhehdjtpsav forum.dlang.org -- Dmitry OlshanskyOf 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.
May 20 2013
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.orgTo 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
20-May-2013 22:14, Idan Arye пишет:On Monday, 20 May 2013 at 16:40:27 UTC, Dmitry Olshansky wrote:2 & 3 do.Long story short - re-read the discussion in the Low-lock thread again: http://forum.dlang.org/thread/pelhvaxwjzhehdjtpsav forum.dlang.orgTo 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 onlypart 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
On Monday, 20 May 2013 at 19:15:34 UTC, Dmitry Olshansky wrote:20-May-2013 22:14, Idan Arye пишет:`hasInstance()` tells you if an instance is already initialized, without returning it and without risking in initializing it if it isn't already 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.Then where you see hasInstance to fit the bill?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.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 `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.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.`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.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.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
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:Saying what it does is not a use-case.20-May-2013 22:14, Idan Arye пишет:`hasInstance()` tells you if an instance is already initialized, without returning it and without risking in initializing it if it isn't already 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.Then where you see hasInstance to fit the bill?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
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 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!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.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.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.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.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
21-May-2013 02:02, Idan Arye пишет:On Monday, 20 May 2013 at 19:15:34 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.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.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?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.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?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
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.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....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.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 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.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.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
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
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
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: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?").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 22 2013
23-May-2013 01:23, Idan Arye пишет:On Tuesday, 21 May 2013 at 17:40:02 UTC, Idan Arye wrote:I've found github pages to be just fine.On Tuesday, 21 May 2013 at 14:58:16 UTC, Idan Arye wrote:OK, I've written and pushed the documentation. I'll try to find somewhere to host the generated HTML.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.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
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
24-May-2013 22:13, Jonathan M Davis пишет:On Friday, May 24, 2013 17:42:59 Dmitry Olshansky wrote: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.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-- Dmitry Olshansky
May 24 2013
On Saturday, May 25, 2013 00:08:37 Dmitry Olshansky wrote:24-May-2013 22:13, Jonathan M Davis пишет: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 DavisLacking 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.
May 24 2013
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
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
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/1294I'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
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: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.OK, I implemented everything and made a pull request: https://github.com/D-Programming-Language/phobos/pull/1294I'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
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: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.On Saturday, 18 May 2013 at 16:58:19 UTC, Idan Arye wrote: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.OK, I implemented everything and made a pull request: https://github.com/D-Programming-Language/phobos/pull/1294I'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