JBossDeveloper

"alesj" wrote:

"smarlow@redhat.com" wrote:

I would like to make a deeper pass of changing some of the calls into ScopeKey. This work will be across different projects (aop and others).

What kind of changes are we talking about here?
Or why is the change to an array not good enough?

Ales,

This can probably be simplified more but I need to test these changes first. I also started to add Javadoc comments. The comment at the top are still rough but gives a little more information. However, this is not the time to review my changes, I need to measure the results first. Of course, feel free to take anything out of here and include in your local ScopeKey.

The link to review the current changes is http://pastebin.com/m516c0bdf

I would like to complete this task soon as this has been taking longer than I wanted and I need to work on other tasks. I would like to integrate this change with the other project branches, but I need to know which branches should be used (preferably the ones that the change would be checked into, which I assume is trunk but wanted to confirm).

I agree that there is risk to changing the projects to accommodate this change but I think we should evaluate that risk after finishing the code changes (but before code is checked in). That way, we know what we are gaining and what we are losing.

Scott

"adrian@jboss.org" wrote:

The brain dead part is that it gets used at runtime in the BasicMetaDataRepository
as the key to a map which it then freezes to avoid mutablity.
The issue being that you still could incur contention because
of the synchronization on the scopes within the ScopeKey traversed in equals()/hashCode().

"adrian@jboss.org" wrote:

2) The other is as I mention above. Use the ScopeKey as the builder class
and retrieve a more optimizated key for internal use within the MDR.

e.g. something like:

public class ScopeKey
{
 ...

 Object getOptimzedKey() {} // Key for use at runtime
}

In fact (1) and (2) are just variations on the same theme, with (2) potentially
being more optimal.

public class ScopeKey
{
 ...

 /**
 * The returned ScopeKey is immutable and optimized for use at runtime.
 * @return Optimized immutable ScopeKey
 */
 public ScopeKey getOptimizedKey()
 {
 return new UnmodifiableScopeKey(this);
 }

}

Updates to ScopeKey are here http://pastebin.com/m2fc19bab

UnmodifiableScopeKey is here http://pastebin.com/m19feda54

Perhaps we could make further MDR changes to reduce the number of times that ScopeKey.equals is called during AS startup. The above changes shouldn't break any existing API and we still pass the serialization ScopeKey unit test. If we agree on making these changes, I'll add unit tests for UnmodifiableScopeKey.

Other changes:

Index: src/main/java/org/jboss/metadata/plugins/repository/basic/BasicMetaDataRepository.java
===================================================================
--- src/main/java/org/jboss/metadata/plugins/repository/basic/BasicMetaDataRepository.java (revision 92394)
+++ src/main/java/org/jboss/metadata/plugins/repository/basic/BasicMetaDataRepository.java (working copy)
@@ -108,8 +108,9 @@
{
if (retrieval == null)
throw new IllegalArgumentException("Null retrieval");
- ScopeKey key = retrieval.getScope();
+ ScopeKey key = retrieval.getScope().getOptimizedKey();
key.freeze();
return retrievals.put(key, retrieval);
}

Index: src/main/java/org/jboss/metadata/plugins/context/AbstractMetaDataContext.java
===================================================================
--- src/main/java/org/jboss/metadata/plugins/context/AbstractMetaDataContext.java (revision 92394)
+++ src/main/java/org/jboss/metadata/plugins/context/AbstractMetaDataContext.java (working copy)
@@ -114,7 +114,8 @@
for (Scope scope : scopes)
key.addScope(scope);
}
- scopeKey = key;
+ scopeKey = key.getOptimizedKey();
}
return scopeKey;
}

"adrian@jboss.org" wrote:

"adrian@jboss.org" wrote:

You do need something like the ScopeKey being mutable until the last minute
because things like the CLASS scope may not be known until the bean instance
is constructed from a factory. Only then do you know the class.

By the way, that's another HACK (and marked as such in the code)
see KernelScopeInfo and InstantiateAction.
If you can think of a better way to do it then feel free to fix it. ;-)

I'm using j.u.c.ConcurrentSkipListMap which depends on Java 6.

As an alternative, we could use the JBoss port (JBCOMMON-83) of ConcurrentSkipListMap.

I'll add some unit tests for the ScopeKey changes.

Unit tests for ScopeKey.getOptimizedKey() are here http://pastebin.com/m4742f057

I added a little more javadoc to ScopeKey which is here http://pastebin.com/m4e6ab9ad

We still need to discuss the use of Java 6 (will help us scale better and Java 5 is hitting end of life October 30, 2009).