Created attachment 9581 [details]
Possible patch

Created attachment 9582 [details]
Complete new source code, for reference

I've made some more changes after talking with Yarda.

1. No longer allocates objects when entering mutex fresh.

2. Now when you enter the read lock from inside a write lock, it
behaves almost like a plain read lock - canWrite() is false,
enterWriteAccess() is forbidden, postReadRequest() runs immediately,
etc. However other threads cannot enter the read lock (until you exit
your outermost write lock). This mode is useful e.g. for firing changes.

3. postWriteRequest now possible again from inside a read mutex. Will
run the runnable in a write lock when you exit the outermost read
lock. In case this outermost read lock was actually inside a write
lock or several, will run it as soon as all other readers have left.
In case it was a pure read lock, will try to acquire the write lock
(may be competing with other threads but this is unavoidable).

Created attachment 9591 [details]
Revised Mutex source

Created attachment 9592 [details]
Revised test diff; now just 2 tests disabled (READ -> WRITE) and one changed (WRITE -> READ -> postReadReq runs immed.)

Plan to also deprecate existing constructors of Mutex and introduce:

public Mutex(int level);
public Mutex(Privileged p, int level);
// no public Mutex(Object lock, int level) - too dangerous

where level is any integer other than Integer.MAX_INT, and EVENT has
an implied level of Integer.MAX_INT. Semantics: if you hold a mutex
with a defined level (read or write), you may not acquire a mutex
(read or write) with an equal or greater defined level, though you may
acquire a mutex (read or write) with a lesser defined level (or no
defined level) according to the normal rules. Should prevent mutex <->
mutex deadlocks.

Implemented lock ordering but have not yet tested it.

Created attachment 9593 [details]
Current state; moduleconfig=slim appears to work normally

1. checkEnter is likely to be implemented easily by ThreadLocal - for
each thread just remember the lowest entered value. That might be faster.

2. implementation of readAccess(Runnable) is dangerous. Imagine what
will be printed by the following code (M1, M2 are mutexes):

class TwoLocks implements Runnable {
  int type;

  public void run () {
    switch (type) {
    case 0:
      type = 1;
      M1.readAccess (this);
      break;
    case 1:
      type = 2;
      M2.readAccess (this);
      break;
    case 2:
      sout ("M1: " + M1.canRead () + " M2: " + M2.canRead ());
    }
  }
}
new TwoLocks ().run ();

will it be true/true or false/true. In my opinion we do not want to
introduce such non-determinism there...

Re. #1: just keeping an int level could work, assuming you changed the
current level both in enter*Access and exit*Access. Good idea. Need to
check how it will work with EVENT. Anyway though I may want to keep a
stack of entered mutexes by thread to ensure that they are entered and
exited in nested order. Such a stack will be about the same speed as
the current threadsToMutexes map (assuming the number of mutexes held
by one thread is small, which is very likely), but using a simple list
is probably better than a WeakSet for memory consumption.

Re. #2: I am not sure what the problem is here. The Javadoc says that
it may be run asynchronously or not and you cannot depend on which. If
you don't like that, don't use that method; use e.g.
readAccess(Mutex.Action).

Created attachment 9611 [details]
Revised Mutex.java

Created attachment 9612 [details]
Revised test patch

Created attachment 9634 [details]
Holds current locks when running r/wAccess(Runnable) asynch

Created attachment 9636 [details]
Revised Mutex.java - when running r/wAccess(Runnable) asynch, sure to acquire same set of locks as are currently held

However now I see another possible trouble area involving ordered
mutexes which it would be better to have solved properly. If you write
this code:

lower.enterReadAccess();
try {
  higher.readAccess(new Runnable()
    public void run() {
      // stuff
    }
  });
} finally {
  lower.exitReadAccess();
}

you might expect that the marked code would be run asynch, since it
cannot be run immediately. Currently it will just give an error.
However generally it is not proper to use readAccess(Runnable) there
since if run asynch it will try to keep the same set of locks it
currently has, which would not make sense here - unless it reordered
them, which could permit it to (asynch) acquire the higher, then the
lower, then run stuff.

On the other hand, it is reasonable to think that this:

lower.enterReadAccess();
try {
  higher.postReadRequest(new Runnable()
    public void run() {
      // stuff
    }
  });
} finally {
  lower.exitReadAccess();
}

would work as expected, running the specified code in a read lock on
higher as soon as it is safe to do so (i.e. during
lower.exitReadAccess(), unless some other locks are already held).

I am not sure whether the current distinction between
readAccess(Runnable) and postReadRequest (similarly
writeAccess(Runnable) and postWriteRequest) makes any sense and is at
all intuitive. For me, it would make sense for readAccess(Runnable) to
always block, just like other overloads of the method, and
postReadRequest to run the runnable at the earliest time when it would
be legal (possibly blocking), and optionally to have some other
methods to run stuff asynch. Unfortunately usage of
Mutex.EVENT.readAccess is to sometimes go asynch, which makes no sense
to me.

Of course the behavior of the old Mutex was never well-specified in
the Javadoc, so it is unclear what existing clients expect.

I am making a branch:

cvs -d $CVSROOTHACK rtag -b -r BLD200304010100 mutex_32439
openide_nowww_1 core_nowww_1 performance_nowww_1

Examining old sources, EVENT behaved as follows:

1. *Access(Mutex.*Action) ran synch if in AWT, else waiting.

2. *Access(Runnable) ran synch if in AWT, else later.

3. post*Request(Runnable) ran asynch in AWT.

I would rather change this to:

1. *Access(Mutex.*Action) as before.

2. *Access(Runnable) as in 1, i.e. synch if in AWT, else waiting.

3. post*Request(Runnable) synch if in AWT, else waiting for AWT as
soon as all ordered mutexes are released (possibly now).

I.e. never use EQ.invokeLater.

Possibly introducing some new AWT_SYNCH constant for the revised
behavior, to keep compatibility for old code, and deprecated AWT.

Need to create code samples for typical use cases involving two nested
systems, e.g. Filesystems + (old) Datasystems, or Filesystems +
Filesystems Extensions (FileObject lookup portion), or Filesystems +
Registry, involving bidirectional change firing etc.

However it is not clear whether there *should* be any cases where two
(public) mutexes are in some relationship to one another. FS + old DS
will hopefully go away. FS-Ext might well just use the same lock as
FS. Registry might have a public lock and treat use of the SFS as an
implementation detail, meaning that non-Registry code should not be
accessing it anyway, and/or might share a lock with FS. In that case
lock ordering on Mutex is probably unnecessary, or a warning could be
printed when any Mutex is acquired while holding another, or when
EQ.invokeAndWait is called while holding any normal Mutex. TBD.

Probably FS + MDR (~ParserDB) is the best example I can think of where
lock ordering and generally Mutex <-> Mutex interaction would be
helpful in 4.0. Maybe there is something in Projects that I don't know
about which has a complex enough  structure to merit this.

Or (re. FS + MDR/ParserDB/JavaHier) there could be a single Mutex for
all "data" inside NB. Then there would be no lock ordering problems.
It would mean you could not e.g. expand a folder while some source was
being parsed, until that file was done. In such a system you need a
way to easily break up big tasks into digestible pieces where possible
and/or to cancel or postpone a task running in a lock.

I've been just talking with David K. He  suggest to add
Mutex.Privileged.getMutex, so he can keep just reference to
Priviledged and return to clients Priviledged.getMutex. The pointer to
Mutex is there anyway, so there is not reason why not expose it.

Created attachment 10106 [details]
Mutex.Privileged.getMutex() patch

What do you think about that Jesse? Does it make sense?

I do not want to go into details but where I would use this is in
Registry SPI where would be method

  Mutex.Priviledged getMutex();

and in API I would give API clients just Mutext by calling
Mutex.Priviledged.getMutex(), but internally in the Registry impl I
would take advantage of Mutex.Priviledged methods.

Yes, M.P.gM() makes perfect sense, I will add it. Thanks for the tip.

Jesse, what do you think about this:

Merged context delegates to list of SPI contexts. Each of its
delegates can have its own Mutex.Priviledged (ie. Mutex.Privileged
RootContext.getMutex()). The merged context needs to return something
like MergedMutexPrivileged[del1.getMutex(), ..., deln.getMutex()]
where enterReadAccess calls enterReadAccess on all delegates, etc.

Do you think something like that is reasonable?

For some time I was thinking about restricting merged context to
accept only contexts with the same Mutex (ie. RootContext). It could
work for some time, but sooner or later somebody will create different
implementation of context and might want to merge it into default
context and then this won't be sufficient anymore - they will have
their own mutexes. Passing everybody the same mutex is also not good -
the context's mutex should be immutable. 

Implementing methods like enterRead() etc. seems to me quite easy.
Methods like postReadRequest() etc. are a bit more complicated but
still doable I think.

Re. Registry API and mutexes.

1. Please don't put Mutex.Privileged into the API. Some threading
models, including the one that I currently find most promising, do
*not* support anything like M.P - i.e. you cannot enter the mutex and
then have control flow return to the caller, you really need to pass
the entire runnable in order for it to work.

2. Keep it simple and assume that all the merged  contexts use the
same mutex. Hopefully in fact they will. Or keep it simpler and define
*the* mutex to use, for any context, as a static constant. Why would
you need more than one?

ad1:
By API in this context you mean also SPI, right? M.P is at the moment
in SPI.RootContext.getMutex(). Will not be difficult to change it
because there is not (and will not be) many impls.

ad2:
I will think about it. At the moment it makes lot of sense to me that
default registry and each project should have its own mutex. If they
all should share the same mutex I would afraid  of potential
performance bottleneck.

>potential performance bottleneck
Lock contention, you mean?
On single-CPU box? Oh, come on!

IMO, individual projects registries and default system registry are
totally independent parts which should not share one lock. But in
reality it might not be an issue.

Re. #1 - right.

Re. #2 - see Petr's reply.

Rather than trying to adjust the behavior of the existing Mutex class
- which has (1) a misleading name, (2) poor support for switchable
implementations, (3) some poorly specified methods like post*Request -
I plan to create alternate lock interfaces in classes in
performance/threaddemo/src/threaddemo/locking/ and use them just in
the thread demo for now. If they seem to be useful enough to include
in the main source base, they can be moved to openide-util.jar. Few
places in the public APIs as of 3.5 refer directly to Mutex anyway;
only Children, I think, and this probably should not need any locking
anyway.

Good plan, at least there will be no regressions. Btw. Another place
where mutex is in API is Compiler. 

If you have some tests that are passing with current impl of Mutex,
please commit them to main test suite. 

Will the old mutex be rewritten to use the new interfaces sometime?

Re. Compilable.MUTEX - OK, but I don't much care about that API. I
wonder if anyone outside the compilation system is even using that field.

Re. new tests - there isn't much new that would apply to the
"traditional" Mutex, i.e. a r/w lock, so it doesn't seem too useful to
backport them.

Re. adapting Mutex to new interfaces - if the new stuff goes into
openide trunk, I could try to provide a method in Mutex:

public Lock asLock();

which would serve as a bridge, or just make it impl the new interface.
However the interfaces do not line up exactly. For example, old Mutex
has no equivalent of canRead/canWrite, and I have no idea how to add
it (because as I may have mentioned, I have no idea how the impl of
old Mutex actually works).

Anyway all this is still provisional - I have yet to see any
demonstration that we actually need read/write locks for anything,
i.e. that we could not just provide some static Object monitor to
synchronize on for APIs that need to work with multiple threads. R/W
locks are only useful if you have heavy usage of the data model in
read mode from multiple simultaneous threads - on a multi-CPU machine,
or on a 1-CPU machine with an aggressive thread scheduler with one of
the threads being the event thread.

Probably best to not touch Mutex at all, just use JDK 5 concurrency tools, and
think about when we can deprecate it. (Tough because of usage from both Children
and ProjectManager.)