[jbosscache-dev] API changes in Habanero

[Ben Wang]

Got a typo in my previous email. What I meant was, "I really like the explicit [api] naming in this case". So I am with you here. :-) 

For the tangent, re: get rid of synchronization on the data map, do we have any evidence on how much speed up we will gain?

-Ben


-----Original Message-----
From: Manik Surtani [mailto:manik at jboss.org] 
Sent: Friday, January 05, 2007 4:30 PM
To: Ben Wang
Cc: jbosscache-dev at lists.jboss.org
Subject: Re: [jbosscache-dev] API changes in Habanero

On 5 Jan 2007, at 01:39, Ben Wang wrote:

> OK, so we are saying XXxDirect belongs in NodeSPI because they are 
> used only for cache behavior customization? But I'd like the explict 
> naming in this case. Previously, you always need to remember to set 
> the option each time before the method call.
>

Not really for cache behaviour customisation at all, but for direct node access.

E.g., Node.get(Object key) could do 2 things:

1) pass a call up the interceptor stack, to make sure the node is locked, cache loaders are considered, etc etc, and then retrieve the value of some data in the node.
2) or it could ignore the interceptor stack and retrieve some data from the node's data map.

In JBC 1.x, (1) was never supported directly from a node.  You had to go through TreeCache to do this.  Node.get() always performed (2).

in 2.0.0.ALPHA1, both (1) and (2) were supported, depending on whether BypassInterceptorChain was set to true as an option.  I explained earlier how this is ugly, cumbersome, and not performant (constant checking of a thread-local variable)

This is why for 2.0.0.ALPHA2, I split such methods into 2 separate ones.  Node.get(Object key) performs (1) above.  NodeSPI.getDirect (Object fqn) performs (2) above.  This also nicely encapsulates direct access from the user API.

I'm not sure what you meant by explicit naming, I thought this is what I have achieved here.  :-)  Previously naming was ambiguous in that the method performed one of two things - (1) or (2) above - based on an option passed in.  Really bad design to rely on a 'global' like that.  :-)


> Another minor issue that I find in my unit test:
> java.lang.UnsupportedOperationException: Cannot directly retrieve 
> children which aren't directly under the current node.
> 	at org.jboss.cache.UnversionedNode.getChildDirect
> (UnversionedNode.java:424)
>
> It seems now:
>
>       NodeSPI node = cache.getRoot().getChildDirect(fqn);
> Only support the retrieval of immediate child. So if, say, I have "/ 
> a/b/c" fqn that I want to retrieve from getRoot(), what is my option 
> then? I'd think to iterate through the Node tree to get a child node 
> is quite tedious and inefficient.
>

Yes, I put this in since I was trying to emulate direct child access for the node in the xxxDirect() methods.  But you are correct, I don't see why this has to be the case and I have changed it in CVS HEAD to not throw this exception and walk the tree internally.

I also need to update the javadocs on the xxxDirect() methods to state that the onus is on the caller to make sure appropriate locks are obtained, etc. as this call does not go through the interceptor chain.  Given that the intent of these methods are for SPI use only, typically from within an interceptor, the user of the SPI would be aware of such constraints and be able to deal with it.



And as a tangent, I was thinking about checking on access for such direct methods and wanted everyone's ideas:

At the moment, xxxDirect() methods that access, for example, the child map or data map of a node are synchronized.  Now this is overkill, but it enforces correctness.  Since we assume that callers to the xxxDirect() methods obtain appropriate locks in all cases, is it safe to remove the synchronization, and instead add a lock- ownership check to such methods.

For example:

public Object getDirect(Object key)
{
	if (!getLock().getReaderOwners().contains(Thread.currentThread()))
		throw new CacheException("Current thread does not have a read lock on node " + fqn);
	return data == null? null : data.get(key); }

This would pretty rightly encapsulate and enforce concurrent access rules on the nodes, even with direct access.  And cheaper than synchronizing the direct access methods.  What do you guys think?

Cheers,
Manik

> Thanks,
>
> -Ben
>
> -----Original Message-----
> From: jbosscache-dev-bounces at lists.jboss.org [mailto:jbosscache-dev- 
> bounces at lists.jboss.org] On Behalf Of Manik Surtani
> Sent: Thursday, January 04, 2007 10:47 PM
> To: jbosscache-dev at lists.jboss.org
> Subject: [jbosscache-dev] API changes in Habanero
>
> Guys,
>
> Here are a few major refactorings I've done on HEAD (checked in 
> already).  Some of these are on the API level and do affect the way 
> interceptors and other subsystems interact with the node structure, 
> hence the email.
>
> * Got rid of the horrible BypassInterceptorChain malarky when 
> interacting with nodes and you don't want calls up the interceptor 
> chain
> 	* Used by interceptors themselves, as well as other internal 
> subsystems including state transfer and the cache itself.
> 	* BypassInterceptorChain will still exist as an Option, as there are 
> some valid use cases for this.
> 		* This option is now SOLELY handled by the 
> InvocationContextInterceptor, which directs the call to the last 
> interceptor in the chain if the option is present
> 		* Not handled internally in Node implementation methods anymore
> 	* Replaced with a bunch of XXXDirect() methods on NodeSPI for direct 
> interaction/bypassing interceptor chains
> 		* E.g., Node.getChild(Fqn f) goes up the interceptor chain, 
> NodeSPI.getChildDirect(Fqn f) operates directly on the node.
> 		* Benefits: much easier to read, understand and maintain, more 
> performant.
> 	
> * Got rid of Node.getNodeSPI()
> 	* NodeSPI interface can now be easily obtained since all methods on 
> NodeSPI which would otherwise return Node now return NodeSPI
> 	* Methods on Cache (such as getRoot()) which would return Node are 
> overridden in CacheSPI to return NodeSPI.
> 	* Gives interceptors and the like access to NodeSPI without letting 
> this interface escape to the user API.
> 	* There are still a very small handful of cases where direct casts 
> are necessary, but this is very internalised (within UnversionedNode 
> and WorkspaceNode, for example) and a bunch of unit tests (for closer
> inspection)
> 	* In general, NodeSPI is now much tighter and can only officially be 
> obtained from another NodeSPI or a CacheSPI.
>
> * Got rid of NodeSPI.getRawData() - superfluous now that we have
> NodeSPI.getDataDirect()
>
> This stuff should give us a much more robust data model and user/ SPI 
> interface for the nodes and caches.
>
> Please let me know what you think or if you have any issues/further 
> suggestions.
>
> Cheers,
> --
> Manik Surtani
>
> Lead, JBoss Cache
> JBoss, a division of Red Hat
>
> Email: manik at jboss.org
> Telephone: +44 7786 702 706
> MSN: manik at surtani.org
> Yahoo/AIM/Skype: maniksurtani
>
>
>
>
> _______________________________________________
> jbosscache-dev mailing list
> jbosscache-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jbosscache-dev