Optional monitors

[yglukhov]

This PR introduces optional monitors feature.
The whole idea is that Object doesn't contain __monitor field anymore.
TypeInfo_Class will hold a monitor offset, if a class has one (by marking class declaration with @monitor attribute), or 0.
Monitor lookup is done with a hash map, if monitorOffset is 0.
The hash map is protected by a primitive RW spin lock.
Monitor finalization will only lookup for monitors, if a monitor was allocated at least once for the type of object being finalized.

druntime counterpart: dlang/druntime#789
discussion: http://forum.dlang.org/thread/xpliectmvwrwthamquke@forum.dlang.org

[bearophile]

This needs a review, to avoid rotting like other pull requests.

[ghost]

This needs a review, to avoid rotting like other pull requests.

Very insightful comment.

Anywho:

• Please remove tabs, use spaces for indentation and keep the indentation consistent. Thanks!
• I think the current code is probably checking for @("monitor") instead of @monitor. There should be an enum monitor; declaration in Druntime that can be used as a UDA since just using @("monitor") could mean arbitrarily anything in user-code.
• There are no test-cases.

[ghost]

I suppose we'll need some kind of __hasMonitor trait to avoid build failures in Druntime, unless someone has a better idea?

[yglukhov]

@AndrejMitrovic, __traits(hasMember, "__monitor") should work.

[ghost]

Yeah, good point. :)

[yglukhov]

I think the current code is probably checking for @("monitor") instead of @monitor. There should be an enum monitor; declaration in Druntime that can be used as a UDA since just using @("monitor") could mean arbitrarily anything in user-code.

Well, my changes to druntime include adding @monitor attrs to some classes. Otherwise the code would not compile there =).

There are no test-cases.

I'm not sure if it needs to be covered explicitly. This is a pretty basic feature, that almost any code should cover it. However i would welcome any good test snippets for that.

[dnadlinger]

Wouldn't the way the code is currently implemented trigger on all structs called monitor? The name is general enough that this might cause problems. For example, I currently have an UDA struct called monitor in a little scientific simulation project of mine (it enables tracking/display of chosen quantities while the simulation is running).

Also, yes, this absolutely needs a test case that checks if the attribute actually causes the monitor field to be emitted. Otherwise, we'll end up with a silent performance regression (and inadvertent ABI change) at some point or another. A simple test could consist of verifying classInstanceSize for a class declaration with and without the monitor field.

[yglukhov]

Wouldn't the way the code is currently implemented trigger on all structs called monitor? The name is general enough that this might cause problems. For example, I currently have an UDA struct called monitor in a little scientific simulation project of mine (it enables tracking/display of chosen quantities while the simulation is running).

You're right. It works exactly as you're saying. But i'm not sure here how to fix that. One way would be using some trickier keyword instead of "monitor".

Also, yes, this absolutely needs a test case that checks if the attribute actually causes the monitor field to be emitted. Otherwise, we'll end up with a silent performance regression (and inadvertent ABI change) at some point or another. A simple test could consist of verifying classInstanceSize for a class declaration with and without the monitor field.

Done in druntime.

[schancel]

I don't quite understand what the benefit of this is over just having a Monitor type? Presumably it allows you to Signal or Wait on any class decorated with @monitor? Why is that much better than having a library Monitor type?

[yglukhov]

@schancel the monitors as they are currently implemented allow to lock/unlock an instance, but not signal/wait. And we already have such type - Mutex =). This PR is aimed is to remove a hidden __monitor field from the Object class, without breaking old behavior.

[yglukhov]

Rebased.

[DmitryOlshansky]

@andralex @WalterBright Please state your view on this subject (again) and let's inact it.

[MartinNowak]

I'm not really happy with the druntime hash table stuff, but I would totally support a deprecation from implicit monitors to an explicit @(Object.Monitor) UDA.

1. recognize monitor UDA and deprecate synchronizing on classes without it
2. templatize all druntime functions that expect Objects to have a monitor
   and dreprecate using them without the UDA
3. wait 6 month
4. stop emitting hidden __monitor for classes without the UDA

This would also deprecate the synchronized (typeid(Object)) anti-pattern.
Adding struct monitor; to object.d for a more concise @monitor would be possible but might break code.

[andralex]

I'm in favor of this. The current backward-compatible approach is the way to go, though I agree with @MartinNowak in the long run we may envision complete deprecation.

Please rebase and let's push this through. Thanks!

[DmitryOlshansky]

Half a year later w/o merging this patch the idea still looks sexy to me.
@MartinNowak any plans to move on this in any of 2 competing plans?

[andralex]

Well what's going on?

[andralex]

"All checks have failed"

[MartinNowak]

Half a year later w/o merging this patch the idea still looks sexy to me.
@MartinNowak any plans to move on this in any of 2 competing plans?

Yes, I'm still opposed to adding a global hash and the outlined plan still makes sense.
Furthermore having monitor support on all classes creates ownership/attribute issue for the monitor (MartinNowak/phobos@8cf0ec2#diff-4e008aedb3026d4a84f58323e53bf017R4883).
I don't have the capacity to pull this story, but starting by adding an @(Object.Monitor) UDA and recognizing that in the compiler should be fairly trivial (e.g. look at the objective-c changes).

[ibuclaw]

recognize monitor UDA and deprecate synchronizing on classes without it

Let's proceed with stage 1 for when 2.071 opens then?

Do we have available documentation / rationale on the deprecated features page? Or a DIP?

[MartinNowak]

Do we have available documentation / rationale on the deprecated features page? Or a DIP?

Sure, a small entry on http://dlang.org/deprecate.html would be nice, a DIP would be overkill IMHO.