[jbosscache-commits] JBoss Cache SVN: r5339 - in core/trunk/src: test/java/org/jboss/cache/api and 1 other directory.

[jbosscache-commits at lists.jboss.org]

Author: manik.surtani at jboss.com
Date: 2008-02-08 13:47:21 -0500 (Fri, 08 Feb 2008)
New Revision: 5339

Modified:
   core/trunk/src/main/java/org/jboss/cache/NodeSPI.java
   core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
   core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java
Log:
Rolled back immutable/defensively copied map performance enhancement

Modified: core/trunk/src/main/java/org/jboss/cache/NodeSPI.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/NodeSPI.java	2008-02-08 18:45:18 UTC (rev 5338)
+++ core/trunk/src/main/java/org/jboss/cache/NodeSPI.java	2008-02-08 18:47:21 UTC (rev 5339)
@@ -40,11 +40,6 @@
  * It is important to node that the direct <b>read</b> methods, such as getDataDirect(), return unmodifiable collections.
  * In addition to being unmodifiable, they are also defensively copied from the underlying data map to ensure view consistency.
  * <p/>
- * <b>Note:</b> the above paragraph was true for JBoss Cache 2.0.0, but for 2.1.0, this has changed to now offer direct view of
- * the data structures, since the defensive copying was seriously affecting scalability.  Please use the *Direct() methods
- * with care - even though you may be able to directly manipulate the data structures returned, the cache isn't designed
- * for you to do so.  Use with care.
- * <p/>
  *
  * @author <a href="mailto:manik at jboss.org">Manik Surtani (manik at jboss.org)</a>
  * @see Node

Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-02-08 18:45:18 UTC (rev 5338)
+++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-02-08 18:47:21 UTC (rev 5339)
@@ -223,8 +223,7 @@
    public Map getDataDirect()
    {
       if (data == null) return Collections.emptyMap();
-      //return Collections.unmodifiableMap(data);
-      return data; // wrapping in an unmodifiable map is too expensive!
+      return Collections.unmodifiableMap(data);
    }
 
    public Object put(Object key, Object value)
@@ -487,8 +486,7 @@
 
    public Set<Object> getChildrenNamesDirect()
    {
-      //return children == null ? Collections.emptySet() : new HashSet<Object>(children.keySet());
-      return children == null ? Collections.emptySet() : children.keySet();
+      return children == null ? Collections.emptySet() : new HashSet<Object>(children.keySet());
    }
 
    public Set<Object> getKeysDirect()
@@ -497,8 +495,7 @@
       {
          return Collections.emptySet();
       }
-//      return Collections.unmodifiableSet(new HashSet<Object>(data.keySet()));
-      return data.keySet();
+      return Collections.unmodifiableSet(new HashSet<Object>(data.keySet()));
    }
 
    public boolean removeChildDirect(Object childName)
@@ -637,8 +634,7 @@
          NodeSPI spi = (NodeSPI) n;
          if (!spi.isDeleted()) exclDeleted.add(spi);
       }
-//      return Collections.unmodifiableSet(exclDeleted);
-      return exclDeleted;
+      return Collections.unmodifiableSet(exclDeleted);
    }
 
    public boolean hasChildrenDirect()
@@ -652,8 +648,7 @@
       {
          if (children != null && !children.isEmpty())
          {
-//            return Collections.unmodifiableSet(new HashSet(children.values()));
-            return new HashSet(children.values());
+            return Collections.unmodifiableSet(new HashSet(children.values()));
          }
          else
          {

Modified: core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java	2008-02-08 18:45:18 UTC (rev 5338)
+++ core/trunk/src/test/java/org/jboss/cache/api/NodeSPITest.java	2008-02-08 18:47:21 UTC (rev 5339)
@@ -83,26 +83,26 @@
       Map dataDirect = root.getDataDirect();
       Set keysDirect = root.getKeysDirect();
 
-//      try
-//      {
-//         dataDirect.remove("k");
-//         fail("getDataDirect() should return an unmodifiable collection object");
-//      }
-//      catch (UnsupportedOperationException uoe)
-//      {
-//         // good; should be immutable
-//      }
-//
-//      try
-//      {
-//         keysDirect.clear();
-//         fail("getKeysDirect() should return an unmodifiable collection object");
-//      }
-//      catch (UnsupportedOperationException uoe)
-//      {
-//         // good; should be immutable
-//      }
+      try
+      {
+         dataDirect.remove("k");
+         fail("getDataDirect() should return an unmodifiable collection object");
+      }
+      catch (UnsupportedOperationException uoe)
+      {
+         // good; should be immutable
+      }
 
+      try
+      {
+         keysDirect.clear();
+         fail("getKeysDirect() should return an unmodifiable collection object");
+      }
+      catch (UnsupportedOperationException uoe)
+      {
+         // good; should be immutable
+      }
+
       // now test defensive copy
       root.put("k2", "v2");
 
@@ -111,7 +111,7 @@
       assertTrue("getKeysDirect() should have made a defensive copy of the data collection object", !keysDirect.contains("k2"));
    }
 
-   public void testChildrenDefensiveCopy()
+   public void testChildrenImmutabilityAndDefensiveCopy()
    {
       // put some stuff in the root node
       String childName = "childName";
@@ -119,7 +119,17 @@
       root.addChild(new Fqn<String>(childName));
       Set childrenDirect = root.getChildrenDirect();
 
-      // test defensive copy
+      try
+      {
+         childrenDirect.clear();
+         fail("getChildrenDirect() should return an unmodifiable collection object");
+      }
+      catch (UnsupportedOperationException uoe)
+      {
+         // good; should be immutable
+      }
+
+      // now test defensive copy
       root.addChild(new Fqn<String>(newChild));
 
       assertTrue("root.addChild() should have succeeded", root.getChildrenNamesDirect().contains(newChild));