www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Optional monitors suggestion

reply "Yuriy" <yuriy.glukhov gmail.com> writes:
Hello, I've played a bit with monitors, trying to make them 
optional, and here's what i've come up with:
https://github.com/yglukhov/dmd/tree/optional_monitor
https://github.com/yglukhov/druntime/tree/optional_monitors

The whole idea is that Object doesn't contain __monitor field 
anymore.
TypeInfo_Class will hold a monitor offset, if a class defines one 
(like void* __monitor), or -1.
Monitor lookup is done with a hash map, if monitorOffset is -1.
The hash map is protected by a primitive RW spin lock.
The only downside i see here is that finalization of every object 
will now lookup for a corresponding monitor, if it's class 
doesn't define embedded one.
Tested on Mac, but i think it should work anywhere.

Your feedback is very appreciated. Thanx.
May 13 2014
next sibling parent Dmitry Olshansky <dmitry.olsh gmail.com> writes:
13-May-2014 19:46, Yuriy пишет:

 Hello, I've played a bit with monitors, trying to make them optional,
 and here's what i've come up with:
 https://github.com/yglukhov/dmd/tree/optional_monitor
 https://github.com/yglukhov/druntime/tree/optional_monitors

 The whole idea is that Object doesn't contain __monitor field anymore.
 TypeInfo_Class will hold a monitor offset, if a class defines one (like
 void* __monitor), or -1.
 Monitor lookup is done with a hash map, if monitorOffset is -1.
 The hash map is protected by a primitive RW spin lock.
 The only downside i see here is that finalization of every object will
 now lookup for a corresponding monitor, if it's class doesn't define
 embedded one.
 Tested on Mac, but i think it should work anywhere.

 Your feedback is very appreciated. Thanx.


Sounds cool, as I always was in favor of pay as you go principle.
Especially with thread-local as default, allocating monitor slot is dubious.


-- 
Dmitry Olshansky
May 13 2014
prev sibling next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 5/13/14, 8:46 AM, Yuriy wrote:

 Hello, I've played a bit with monitors, trying to make them optional,
 and here's what i've come up with:
 https://github.com/yglukhov/dmd/tree/optional_monitor
 https://github.com/yglukhov/druntime/tree/optional_monitors

 The whole idea is that Object doesn't contain __monitor field anymore.
 TypeInfo_Class will hold a monitor offset, if a class defines one (like
 void* __monitor), or -1.
 Monitor lookup is done with a hash map, if monitorOffset is -1.
 The hash map is protected by a primitive RW spin lock.
 The only downside i see here is that finalization of every object will
 now lookup for a corresponding monitor, if it's class doesn't define
 embedded one.
 Tested on Mac, but i think it should work anywhere.

 Your feedback is very appreciated. Thanx.


I think that's a great idea. Please follow through with it! -- Andrei
May 13 2014
prev sibling next sibling parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
 The only downside i see here is that finalization of every 
 object will now lookup for a corresponding monitor, if it's 
 class doesn't define embedded one.


Ok, i think i've found a simple yet effective solution.
Adding a flag to ClassFlags: hasAllocatedMonitors.
Mark m_flags with this flag in runtime, when monitor is allocated.
When finalizing, check this flag to know if we need to lookup the 
monitors.

This way the only types that suffer would be those who don't 
define embedded __monitor, but still use synchronized() over them.

Would that be an ultimate win-win solution?
May 13 2014
parent "Yuriy" <yuriy.glukhov gmail.com> writes:
 Ok, i think i've found a simple yet effective solution.


Implemented and pushed to the repo. Any more thoughts before a PR?
May 14 2014
prev sibling next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Yuriy:


 Your feedback is very appreciated. Thanx.


What's the syntax to define a class without its __monitor? Are 
you using an annotation like  no_monitor?

Bye,
bearophile
May 14 2014
parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
On Wednesday, 14 May 2014 at 08:37:41 UTC, bearophile wrote:

 What's the syntax to define a class without its __monitor? Are 
 you using an annotation like  no_monitor?


No syntax for that. The __monitor is not present by default 
anywhere. If you need it, you need to define one.

class A
{

}

class B
{
     void* __monitor;
}

{
     A a = new A(); // sizeof a instance is 8
     B b = new B(); // sizeof b instance is 16
     synchronized(a)
     {
         // Internally allocates a monitor and place it to global 
monitor hash map. On synchronize, search the hash map for already 
created monitors.
     }

     synchronized(b)
     {
         // Internally allocates a monitor, and sets b.__monitor 
to it. Pretty much like it was done before, except that __monitor 
field may now have different offset.
     }
}

So the semantics remains the same, you just may want to define a 
__monitor field if you want better performance.
May 14 2014
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Yuriy:


 No syntax for that. The __monitor is not present by default 
 anywhere. If you need it, you need to define one.



 class B
 {
     void* __monitor;


What kind of clear error messages do you receive if you write:

class B {
     void* _monitor;

Or:

class B {
     size_t __monitor;


This isn't C++, in D land we prefer features to be safe and 
clean, the D compiler is designed to have clean syntax, to give 
nice errors, and remove the probability of invisible mistakes as 
much as possible. An annotation like  no_monitor (or its opposite 
 monitor if we don't want __monitor on default) seems the safe 
and clean way to ask for this in D.

Bye,
bearophile
May 14 2014
parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
bearophile, good point. What do you think of the following 
solution:
1. Declaring member variable with __monitor identifier is 
disallowed.
2. class may be defined with  __monitor attribute, in which case 
a __monitor variable will be added to it's beginning.
3. Subclass of such class may again be defined with  __monitor 
attar, in which case it will be simply ignored, as it already 
inherits __monitor from super.
May 14 2014
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Yuriy:


 2. class may be defined with  __monitor attribute, in which 
 case a __monitor variable will be added to it's beginning.


I suggest a name like " monitor". The is no need for the 
underscores.



 3. Subclass of such class may again be defined with  __monitor 
 attar, in which case it will be simply ignored, as it already 
 inherits __monitor from super.


A more tidy design could require the  monitor in all classes that 
inherit from a  monitor-annotated class. But this also looks a 
little overkill. So let's hear what other people think about this.

Bye,
bearophile
May 14 2014
next sibling parent "Yuriy" <yuriy.glukhov gmail.com> writes:
On Wednesday, 14 May 2014 at 11:14:47 UTC, bearophile wrote:

 I suggest a name like " monitor". The is no need for the 
 underscores.


Done.


 A more tidy design could require the  monitor in all classes 
 that inherit from a  monitor-annotated class. But this also 
 looks a little overkill. So let's hear what other people think 
 about this.


It should be easy to implement, but i'm not sure if it will be 
handy. Monitors are not something that affects children behavior 
too much. So changing a superclass to be  monitored should not 
require updating subclasses. IMHO.
May 14 2014
prev sibling parent "Dejan Lekic" <dejan.lekic gmail.com> writes:
Actually, I prefer your original proposal  no_monitor as I expect 
D classes to have monitors by default, until D eventually 
changes. Having  monitor implies that by default there are no 
monitors (which makes sense, but unfortunatelly that is not the 
current state of affairs in D).
May 14 2014
prev sibling next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Yuriy:


 2. class may be defined with  __monitor attribute, in which 
 case a __monitor variable will be added to it's beginning.


I'd like to know why you think D classes should not have the 
monitor on default (this means why you don't plan for a 
 no_pointer).

Currently D classes have that pointer on default, so I think the 
exchange of the default should be justified with an explanation.

Most D classes are currently allocated on the heap by the D GC 
that has a minimal allocation size, so the save in memory is 
small.

Bye,
bearophile
May 14 2014
parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
On Wednesday, 14 May 2014 at 13:07:32 UTC, bearophile wrote:

 I'd like to know why you think D classes should not have the 
 monitor on default (this means why you don't plan for a 
  no_pointer).


There are 4 reasons for that.
1. I'm thinking of monitors as members of Object class, so all 
other classes have monitors just because they are inheriting it. 
So with my proposal you would be able to add a monitor, but not 
remove it.
2. Mutex synchronization is pretty old-school approach, there's a 
lot of cons to using it, and thus it should not be carved into 
the language core.
3. I can't imagine a project that has more than 10% of it's 
classes being synchronized on, so this feature looks like more 
often unneeded than needed.
4. I consider D a killemall language, that may be potentially 
used on tiny AVRs and PICs, where polymorphism might be welcome, 
but an extra pointer for each class instance may become a 
blocker. I know, thats fantasy now, but i think it's crucial to 
keep this concept of D.
May 14 2014
next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Yuriy:


 4. I consider D a killemall language, that may be potentially 
 used on tiny AVRs and PICs, where polymorphism might be 
 welcome, but an extra pointer for each class instance may 
 become a blocker. I know, thats fantasy now, but i think it's 
 crucial to keep this concept of D.


It's wiser to not dream too much :-)

I leave the more complex parts of this discussion to people that 
know the topic more than me.

Bye,
bearophile
May 14 2014
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 5/14/14, 6:33 AM, Yuriy wrote:

 On Wednesday, 14 May 2014 at 13:07:32 UTC, bearophile wrote:

 I'd like to know why you think D classes should not have the monitor
 on default (this means why you don't plan for a  no_pointer).


 There are 4 reasons for that.
 1. I'm thinking of monitors as members of Object class, so all other
 classes have monitors just because they are inheriting it. So with my
 proposal you would be able to add a monitor, but not remove it.


Agreed.


 2. Mutex synchronization is pretty old-school approach, there's a lot of
 cons to using it, and thus it should not be carved into the language core.


Agreed.


 3. I can't imagine a project that has more than 10% of it's classes
 being synchronized on, so this feature looks like more often unneeded
 than needed.


Agreed.


 4. I consider D a killemall language, that may be potentially used on
 tiny AVRs and PICs, where polymorphism might be welcome, but an extra
 pointer for each class instance may become a blocker. I know, thats
 fantasy now, but i think it's crucial to keep this concept of D.


Agreed at least with the "killing the mall" part :o).


Andrei
May 14 2014
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 5/14/2014 2:17 PM, Andrei Alexandrescu wrote:

 On 5/14/14, 6:33 AM, Yuriy wrote:

 4. I consider D a killemall language, that may be potentially used on
 tiny AVRs and PICs, where polymorphism might be welcome, but an extra
 pointer for each class instance may become a blocker. I know, thats
 fantasy now, but i think it's crucial to keep this concept of D.


 Agreed at least with the "killing the mall" part :o).


While I agree with Andrei's agreements (!), the rationale for the current 
approach is to make it relatively straightforward to translate existing Java 
code into D. There was a fair amount of this in the early days of D, I'm not 
sure how much of that lately.
May 17 2014
next sibling parent "Yuriy" <yuriy.glukhov gmail.com> writes:
On Sunday, 18 May 2014 at 05:01:21 UTC, Walter Bright wrote:

 While I agree with Andrei's agreements (!), the rationale for 
 the current approach is to make it relatively straightforward 
 to translate existing Java code into D. There was a fair amount 
 of this in the early days of D, I'm not sure how much of that 
 lately.



looking familiar, when possible. And i think it has to continue 
being like so, until D conquers the world =). As to 
builtin/optional monitors currently my PRs do not change anything 
in that sense.

But even if we later deprecate synchronizing on non- monitored 
classes, it still remains trivial to fix when porting from Java.
May 17 2014
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 5/17/14, 10:01 PM, Walter Bright wrote:

 On 5/14/2014 2:17 PM, Andrei Alexandrescu wrote:

 On 5/14/14, 6:33 AM, Yuriy wrote:

 4. I consider D a killemall language, that may be potentially used on
 tiny AVRs and PICs, where polymorphism might be welcome, but an extra
 pointer for each class instance may become a blocker. I know, thats
 fantasy now, but i think it's crucial to keep this concept of D.


 Agreed at least with the "killing the mall" part :o).


 While I agree with Andrei's agreements (!), the rationale for the
 current approach is to make it relatively straightforward to translate
 existing Java code into D. There was a fair amount of this in the early
 days of D, I'm not sure how much of that lately.


Maybe I misunderstood - I thought the change preserves semantics. -- Andrei
May 18 2014
next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Andrei Alexandrescu:


 Maybe I misunderstood - I thought the change preserves 
 semantics. -- Andrei


Perhaps it's Walter that has misunderstood.

Bye,
bearophile
May 18 2014
prev sibling next sibling parent reply "David Nadlinger" <code klickverbot.at> writes:
On Sunday, 18 May 2014 at 10:33:53 UTC, Andrei Alexandrescu wrote:

 Maybe I misunderstood - I thought the change preserves 
 semantics. -- Andrei


There are two layers to the changes discussed in this thread. The 
first is to remove __monitor from Object. This is something I 
think we all agree on.

Now there are two possibilities for what to do when a user tries 
to synchronize on an instance that does not have a monitor field. 
One option would be to just fall back on a global lock lookup 
table of some sorts. This is what Yuriy's proposal does, and 
indeed preserves language semantics at the expense of a "silent" 
slowdown in these cases. The other is to outright forbid 
synchronizing on such objects. This is a small breaking change, 
but arguably the cleaner solution.

If we didn't have to worry about being backwards compatible, I'd 
definitely argue for the second solution. Java compatibility is 
not a very strong argument in my opinion. First, porting a Java 
application 1:1 is asking for  performance hazards (w.r.t. GC, 
...) anyway. Second, the no-synchronized-by-default design allows 
for clear error messages that immediately suggest the correct fix 
(add an attribute to the class declaration), and for mechanical 
porting, Java classes could just be translated to 
" synchronizable class" or whatever.

David
May 18 2014
next sibling parent David Gileadi <gileadis NSPMgmail.com> writes:
On 5/18/14, 7:10 AM, David Nadlinger wrote:

 ... Java classes could just be
 translated to " synchronizable class" or whatever.


Whether we preserve backwards compatibility or not,  synchronizable has 
my vote for the bikeshed's color.
May 19 2014
prev sibling parent "Martin Nowak" <code dawg.eu> writes:
 If we didn't have to worry about being backwards compatible, 
 I'd definitely argue for the second solution. Java 
 compatibility is not a very strong argument in my opinion. 
 First, porting a Java application 1:1 is asking for  
 performance hazards (w.r.t. GC, ...) anyway. Second, the 
 no-synchronized-by-default design allows for clear error 
 messages that immediately suggest the correct fix (add an 
 attribute to the class declaration), and for mechanical 
 porting, Java classes could just be translated to 
 " synchronizable class" or whatever.


I think deprecating the old behavior is the right choice here.
May 20 2014
prev sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sun, 18 May 2014 06:33:55 -0400, Andrei Alexandrescu  
<SeeWebsiteForEmail erdani.org> wrote:


 On 5/17/14, 10:01 PM, Walter Bright wrote:

 On 5/14/2014 2:17 PM, Andrei Alexandrescu wrote:

 On 5/14/14, 6:33 AM, Yuriy wrote:

 4. I consider D a killemall language, that may be potentially used on
 tiny AVRs and PICs, where polymorphism might be welcome, but an extra
 pointer for each class instance may become a blocker. I know, thats
 fantasy now, but i think it's crucial to keep this concept of D.


 Agreed at least with the "killing the mall" part :o).


 While I agree with Andrei's agreements (!), the rationale for the
 current approach is to make it relatively straightforward to translate
 existing Java code into D. There was a fair amount of this in the early
 days of D, I'm not sure how much of that lately.


 Maybe I misunderstood - I thought the change preserves semantics. --


You didn't misunderstand. The change will use an external monitor stored  
in a global hashtable if an internal monitor does not exist.  
synchronized(obj) will always work, even if the object does not contain a  
monitor field.

-Steve
May 19 2014
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 18/05/14 07:01, Walter Bright wrote:


 While I agree with Andrei's agreements (!), the rationale for the
 current approach is to make it relatively straightforward to translate
 existing Java code into D. There was a fair amount of this in the early
 days of D, I'm not sure how much of that lately.


DWT is still around. Although, I don't have any memory of seeing the 
monitor being used.

-- 
/Jacob Carlborg
May 18 2014
parent Jacob Carlborg <doob me.com> writes:
On 19/05/14 08:29, Jacob Carlborg wrote:


 DWT is still around. Although, I don't have any memory of seeing the
 monitor being used.


The "synchronized" statement is used in DWT.

-- 
/Jacob Carlborg
May 18 2014
prev sibling parent reply "Damian Day" <damianroyday gmail.com> writes:
On Wednesday, 14 May 2014 at 11:06:43 UTC, Yuriy wrote:

 bearophile, good point. What do you think of the following 
 solution:
 1. Declaring member variable with __monitor identifier is 
 disallowed.
 2. class may be defined with  __monitor attribute, in which 
 case a __monitor variable will be added to it's beginning.
 3. Subclass of such class may again be defined with  __monitor 
 attar, in which case it will be simply ignored, as it already 
 inherits __monitor from super.


These would be breaking changes. I see the benefit but...

If the compiler recognizes the class using synchronized, it could
use a little magic and add a monitor variable itself, this would
not cause any breaking change.
May 14 2014
parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
On Wednesday, 14 May 2014 at 13:53:51 UTC, Damian Day wrote:

 These would be breaking changes. I see the benefit but...


Breaking, only if someone used to define __monitor in his class. 
Which is kinda weird, and according to docs (identifiers starting 
with __ should are reserved). Or if someone used to access 
__monitor field of something, which is also kinda weird. 
Otherwise, no breaking changes here.


 If the compiler recognizes the class using synchronized, it 
 could
 use a little magic and add a monitor variable itself, this would
 not cause any breaking change.


I guess it would need a whole lot of magic, since synchronized() 
is normally used outside of class and even its module, except for 
synchronized(this) or smth. And adding members in compile-time to 
classes defined in not_current_module sounds like a bad idea.
However, compiler could do some little magic, adding __monitor to 
a class, if it can prove that synchronized is used in the same 
module where class is defined.
Another however, i'm not sure if such optimization is even worth 
thinking of, since you still can sync on any class regardless it 
defines __monitor or not.
May 14 2014
parent reply "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Wednesday, 14 May 2014 at 14:07:50 UTC, Yuriy wrote:

 On Wednesday, 14 May 2014 at 13:53:51 UTC, Damian Day wrote:

 These would be breaking changes. I see the benefit but...


 Breaking, only if someone used to define __monitor in his 
 class. Which is kinda weird, and according to docs (identifiers 
 starting with __ should are reserved). Or if someone used to 
 access __monitor field of something, which is also kinda weird. 
 Otherwise, no breaking changes here.


The object's layout and size also change, but the layout of 
objects is implementation defined AFAIK, and its size shouldn't 
be hard-coded anyway, so the breakage is minimal.
May 14 2014
parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
On Wednesday, 14 May 2014 at 14:44:07 UTC, Marc Schütz wrote:

 The object's layout and size also change, but the layout of 
 objects is implementation defined AFAIK, and its size shouldn't 
 be hard-coded anyway, so the breakage is minimal.


You're right. Shared libraries will need to be rebuilt.

Meanwhile, i've updated the PRs according to latest notes. 
 monitor attr, forbid __monitor declaration, so everyone can have 
some fun with it. =)
May 14 2014
parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Yuriy:


 Meanwhile, i've updated the PRs according to latest notes. 
  monitor attr, forbid __monitor declaration, so everyone can 
 have some fun with it. =)


You are good. Before this is ready for Walter's judgement I think 
this needs some simple performance/memory benchmarks, to compare 
the situation before and after this change.

Bye,
bearophile
May 14 2014
parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
On Wednesday, 14 May 2014 at 19:08:30 UTC, bearophile wrote:

 You are good. Before this is ready for Walter's judgement I 
 think this needs some simple performance/memory benchmarks, to 
 compare the situation before and after this change.


I've made some minor optimizations, and rebased everything on 
current master (both dmd and druntime).
Also here is a simple test i've run:
http://dpaste.dzfl.pl/f44762a17fe4

You-have-no-choice-but-pay-the-price-monitors:
max: 8709, min: 3227, average: 3484

Optional inlined monitor:
max: 12010, min: 5136, average: 5361

Optional missing monitor: no need to measure for obvious reasons.

So yeah, my monitors are like %50 slower, but
1. does it really matter?
2. i think further optimizations may be done here. The best way i 
see it is to make _d_monitorenter/exit to be templates so that 
monitored classes will be at least as fast as current monitors, 
and even faster if _d_monitorenter/exit will be inlined. But 
anyway such optimization may be done later if needed, without any 
code breaking changes.
May 16 2014
parent "bearophile" <bearophileHUGS lycos.com> writes:
Yuriy:


 http://dpaste.dzfl.pl/f44762a17fe4


As workaround for a D design bug I suggest to write code like 
this:
foreach (immutable c; 0 .. 100_000)

Instead of:
foreach(c; 0..100000)

Because unfortunately by default that c index is not immutable, 
and this causes significant troubles if you mutate it by mistake.

Bye,
bearophile
May 16 2014
prev sibling parent reply "Martin Nowak" <code dawg.eu> writes:
On Wednesday, 14 May 2014 at 09:16:21 UTC, Yuriy wrote:

 On Wednesday, 14 May 2014 at 08:37:41 UTC, bearophile wrote:

 What's the syntax to define a class without its __monitor? Are 
 you using an annotation like  no_monitor?


 No syntax for that. The __monitor is not present by default 
 anywhere. If you need it, you need to define one.

 class A
 {

 }

 class B
 {
     void* __monitor;
 }

 {
     A a = new A(); // sizeof a instance is 8
     B b = new B(); // sizeof b instance is 16
     synchronized(a)
     {
         // Internally allocates a monitor and place it to 
 global monitor hash map. On synchronize, search the hash map 
 for already created monitors.


Global hashmap is a bad idea IMO because it's possibly expensive 
and impure. Rather deprecate synchronizing on classes without an 
explicit monitor.


     }

     synchronized(b)
     {
         // Internally allocates a monitor, and sets b.__monitor 
 to it. Pretty much like it was done before, except that 
 __monitor field may now have different offset.
     }


A UDA on the class is cleaner than adding magic members.

 }

 So the semantics remains the same, you just may want to define 
 a __monitor field if you want better performance.
May 17 2014
parent "Yuriy" <yuriy.glukhov gmail.com> writes:
On Sunday, 18 May 2014 at 04:44:56 UTC, Martin Nowak wrote:

 Global hashmap is a bad idea IMO because it's possibly 
 expensive and impure. Rather deprecate synchronizing on classes 
 without an explicit monitor.


Totally agreed. Hashmap is done just not to break existing code. 
I don't think i've got the right to deprecate anything with my 
PRs =).


 A UDA on the class is cleaner than adding magic members.


Yes, my PRs already include that.
May 17 2014
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 5/13/2014 8:46 AM, Yuriy wrote:

 Your feedback is very appreciated. Thanx.


The "_monitor" slot is also used for std.signals. It's been set up in druntime 
to support more than just being a monitor.

We've also considered it for a hook for a reference count (though that design 
had other problems).

I'm not saying your design is wrong, just that we should consider what to do 
with these other issues.
May 18 2014
parent reply "Yuriy" <yuriy.glukhov gmail.com> writes:
On Sunday, 18 May 2014 at 19:57:34 UTC, Walter Bright wrote:

 The "_monitor" slot is also used for std.signals. It's been set 
 up in druntime to support more than just being a monitor.

 We've also considered it for a hook for a reference count 
 (though that design had other problems).

 I'm not saying your design is wrong, just that we should 
 consider what to do with these other issues.


My current PR doesn't affect that also. The signals and 
finalization callbacks do work as they used to. What changes is 
just monitors are looked up in an external hash table, when they 
are not declared inside the class, by applying  monitor attr to 
it. synchronized blocks are also not affected in the same way.
However, if you're planning to use monitors for reference 
counting, such external lookup may become a huge performance 
issue. But current reference counts of monitors themselves are 
not an issue at all.

I'm on my way to DConf now, so we can talk about details there if 
you wish.
May 19 2014
parent reply "Martin Nowak" <code dawg.eu> writes:
On Monday, 19 May 2014 at 07:11:41 UTC, Yuriy wrote:

 On Sunday, 18 May 2014 at 19:57:34 UTC, Walter Bright wrote:

 The "_monitor" slot is also used for std.signals. It's been 
 set up in druntime to support more than just being a monitor.

 We've also considered it for a hook for a reference count 
 (though that design had other problems).

 I'm not saying your design is wrong, just that we should 
 consider what to do with these other issues.


 My current PR doesn't affect that also. The signals and 
 finalization callbacks do work as they used to. What changes is 
 just monitors are looked up in an external hash table, when 
 they are not declared inside the class, by applying  monitor 
 attr to it. synchronized blocks are also not affected in the 
 same way.
 However, if you're planning to use monitors for reference 
 counting, such external lookup may become a huge performance 
 issue. But current reference counts of monitors themselves are 
 not an issue at all.

 I'm on my way to DConf now, so we can talk about details there 
 if you wish.


I don't see why we need to introduce a global hashtable 
(performance impact, not pure). We could warn/deprecate/remove 
synchronizing on classes without the  monitor attribute.
May 21 2014
next sibling parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Thu, 22 May 2014 02:48:16 -0400, Martin Nowak <code dawg.eu> wrote:


 On Monday, 19 May 2014 at 07:11:41 UTC, Yuriy wrote:

 On Sunday, 18 May 2014 at 19:57:34 UTC, Walter Bright wrote:

 The "_monitor" slot is also used for std.signals. It's been set up in  
 druntime to support more than just being a monitor.

 We've also considered it for a hook for a reference count (though that  
 design had other problems).

 I'm not saying your design is wrong, just that we should consider what  
 to do with these other issues.


 My current PR doesn't affect that also. The signals and finalization  
 callbacks do work as they used to. What changes is just monitors are  
 looked up in an external hash table, when they are not declared inside  
 the class, by applying  monitor attr to it. synchronized blocks are  
 also not affected in the same way.
 However, if you're planning to use monitors for reference counting,  
 such external lookup may become a huge performance issue. But current  
 reference counts of monitors themselves are not an issue at all.

 I'm on my way to DConf now, so we can talk about details there if you  
 wish.


 I don't see why we need to introduce a global hashtable (performance  
 impact, not pure). We could warn/deprecate/remove synchronizing on  
 classes without the  monitor attribute.


A possible path is to introduce the change, but put  monitor on Object.  
This will allow all current code to compile as-is.

Then users who are concerned about their code being affected would be able  
to remove  monitor from Object, recompile druntime and phobos (once we  
make it work) and allow people to see how their code breaks without being  
left hanging (the shipping compiler would still have  monitor).

Then eventually, we can remove  monitor from Object, and users who still  
wish to force  monitor on Object can do so (recompile druntime and phobos  
with  monitor added).

-Steve
May 22 2014
parent reply "Andrej Mitrovic" <andrej.mitrovich gmail.com> writes:
On Thursday, 22 May 2014 at 17:25:30 UTC, Steven Schveighoffer 
wrote:

 A possible path is to introduce the change, but put  monitor on 
 Object. This will allow all current code to compile as-is.

 Then users who are concerned about their code being affected 
 would be able to remove  monitor from Object, recompile 
 druntime and phobos (once we make it work) and allow people to 
 see how their code breaks without being left hanging (the 
 shipping compiler would still have  monitor).

 Then eventually, we can remove  monitor from Object, and users 
 who still wish to force  monitor on Object can do so (recompile 
 druntime and phobos with  monitor added).


This sounds like a reasonable deprecation stage.

Anyway it seems like we're all mostly on the same page here 
(remove the monitor) except the final part: potential implicit 
performance penalty vs code breakage after a deprecation stage. 
Either way we chose there's a pull ready[1] that's going stale.

Can we get an update from both Andrei & Walter to get going with 
this?

[1]: https://github.com/D-Programming-Language/dmd/pull/3547
Sep 08 2014
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 9/8/14, 2:10 AM, Andrej Mitrovic wrote:

 On Thursday, 22 May 2014 at 17:25:30 UTC, Steven Schveighoffer wrote:

 A possible path is to introduce the change, but put  monitor on
 Object. This will allow all current code to compile as-is.

 Then users who are concerned about their code being affected would be
 able to remove  monitor from Object, recompile druntime and phobos
 (once we make it work) and allow people to see how their code breaks
 without being left hanging (the shipping compiler would still have
  monitor).

 Then eventually, we can remove  monitor from Object, and users who
 still wish to force  monitor on Object can do so (recompile druntime
 and phobos with  monitor added).


 This sounds like a reasonable deprecation stage.

 Anyway it seems like we're all mostly on the same page here (remove the
 monitor) except the final part: potential implicit performance penalty
 vs code breakage after a deprecation stage. Either way we chose there's
 a pull ready[1] that's going stale.

 Can we get an update from both Andrei & Walter to get going with this?

 [1]: https://github.com/D-Programming-Language/dmd/pull/3547


I'm in favor of getting rid of the monitor field. I'm not sure I 
understand whether  monitor introduces a field or just an entry in the 
hashtable. FWIW we could do this for backward compatibility: objects 
without  monitor use the hashtable, and those with  monitor use a field.

Andrei
Sep 08 2014
next sibling parent "Yuriy" <yuriy.glukhov gmail.com> writes:
On Monday, 8 September 2014 at 16:56:49 UTC, Andrei Alexandrescu
wrote:

 FWIW we could do this for backward compatibility: objects 
 without  monitor use the hashtable, and those with  monitor use 
 a field.


And that's exactly what current PR suggests. However, it's
stalled due to another issue. The implementation relies on
patching TypeInfo_Class in runtime, by adding a bit flag to the
TypeInfos of classes for which a monitor was allocated in the
hash-table at least once. This is done to tell GC (on
finalization) not to lookup monitors for classes that never used
it. So again, the main issue is modification of TypeInfo in
runtime, whereas it's supposed to be read-only. That's not my
opinion, I'm totally fine with writing a bit to "read-only"
memory. =)
Sep 10 2014
prev sibling parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Mon, 08 Sep 2014 12:57:13 -0400, Andrei Alexandrescu  
<SeeWebsiteForEmail erdani.org> wrote:


 On 9/8/14, 2:10 AM, Andrej Mitrovic wrote:

 On Thursday, 22 May 2014 at 17:25:30 UTC, Steven Schveighoffer wrote:

 A possible path is to introduce the change, but put  monitor on
 Object. This will allow all current code to compile as-is.

 Then users who are concerned about their code being affected would be
 able to remove  monitor from Object, recompile druntime and phobos
 (once we make it work) and allow people to see how their code breaks
 without being left hanging (the shipping compiler would still have
  monitor).

 Then eventually, we can remove  monitor from Object, and users who
 still wish to force  monitor on Object can do so (recompile druntime
 and phobos with  monitor added).


 This sounds like a reasonable deprecation stage.

 Anyway it seems like we're all mostly on the same page here (remove the
 monitor) except the final part: potential implicit performance penalty
 vs code breakage after a deprecation stage. Either way we chose there's
 a pull ready[1] that's going stale.




Wow, thanks for waiting until I had some breathing time to look at D to  
bring this up again ;)

I think the implicit performance penalty is not hard to fix.

First, you can flag such uses "You know, you are synchronizing on a  
non- monitor object, this will be expensive, may want to put  monitor on  
that bad boy" with a tool/compiler.

Second, the number of *classes* upon which one uses synchronization is  
very very small. I anticipate, you will have to put  monitor on 1-2 types  
in your hierarchy, and then everything works beautifully.


 Can we get an update from both Andrei & Walter to get going with this?

 [1]: https://github.com/D-Programming-Language/dmd/pull/3547


 I'm in favor of getting rid of the monitor field. I'm not sure I  
 understand whether  monitor introduces a field or just an entry in the  
 hashtable. FWIW we could do this for backward compatibility: objects  
 without  monitor use the hashtable, and those with  monitor use a field.


 monitor means to do exactly what the compiler does now. Essentially,  
 monitor tags any class and its derivatives with the existing behavior  
(and flags the TypeInfo as such I think). Without  monitor, you have the  
new behavior which has no monitor, but uses the global hashtable if you  
happen to synchronize with it.

-Steve
Sep 16 2014
parent reply "Sean Kelly" <sean invisibleduck.org> writes:
Yeah I haven't looked at the implementation yet either.
Currently, it's possible to set a core.sync.Mutex as an object
monitor.  Would this capability be preserved?  Someone mentioned
a hashtable of monitors... do we really need this?  I can see the
desire to preserve existing semantics and so a user shouldn't
have to explicitly construct a monitor to use synchronized since
they don't today, but it should still be possible to do so if the
user has some need to.
Sep 16 2014
parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Tue, 16 Sep 2014 15:01:17 -0400, Sean Kelly <sean invisibleduck.org>  
wrote:


 Yeah I haven't looked at the implementation yet either.
 Currently, it's possible to set a core.sync.Mutex as an object
 monitor.  Would this capability be preserved?  Someone mentioned
 a hashtable of monitors... do we really need this?  I can see the
 desire to preserve existing semantics and so a user shouldn't
 have to explicitly construct a monitor to use synchronized since
 they don't today, but it should still be possible to do so if the
 user has some need to.


I would assume you couldn't unless the class is marked  monitor. I don't  
see a problem with this. The proposed migration path starts with all  
objects being marked with  monitor.

-Steve
Sep 16 2014
prev sibling parent "Yuriy" <yuriy.glukhov gmail.com> writes:
On Thursday, 22 May 2014 at 06:48:17 UTC, Martin Nowak wrote:

 I don't see why we need to introduce a global hashtable 
 (performance impact, not pure). We could warn/deprecate/remove 
 synchronizing on classes without the  monitor attribute.


It's not only about synchronization. Signals use monitors to 
receive finalization callbacks.
May 22 2014
prev sibling parent "deadalnix" <deadalnix gmail.com> writes:
On Tuesday, 13 May 2014 at 15:46:51 UTC, Yuriy wrote:

 Hello, I've played a bit with monitors, trying to make them 
 optional, and here's what i've come up with:
 https://github.com/yglukhov/dmd/tree/optional_monitor
 https://github.com/yglukhov/druntime/tree/optional_monitors

 The whole idea is that Object doesn't contain __monitor field 
 anymore.
 TypeInfo_Class will hold a monitor offset, if a class defines 
 one (like void* __monitor), or -1.
 Monitor lookup is done with a hash map, if monitorOffset is -1.
 The hash map is protected by a primitive RW spin lock.
 The only downside i see here is that finalization of every 
 object will now lookup for a corresponding monitor, if it's 
 class doesn't define embedded one.
 Tested on Mac, but i think it should work anywhere.

 Your feedback is very appreciated. Thanx.


Why not delete it altogether and define a Locakable constraint 
for thing that are Locakable. Only these can be used in 
synchronize.

This will cause to go through 2 indirections for something that 
won't be used on most objects as they are not shared anyway.
Sep 16 2014