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

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

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. =)

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

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.

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

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.