You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by tv...@apache.org on 2016/10/13 14:25:04 UTC

svn commit: r1764695 - in /commons/proper/jcs/trunk/commons-jcs-core/src: main/java/org/apache/commons/jcs/auxiliary/disk/indexed/ main/java/org/apache/commons/jcs/utils/struct/ test/java/org/apache/commons/jcs/utils/struct/

Author: tv
Date: Thu Oct 13 14:25:04 2016
New Revision: 1764695

URL: http://svn.apache.org/viewvc?rev=1764695&view=rev
Log:
Fix SortedPreferentialArray to actually do what was is supposed to do

Modified:
    commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/indexed/IndexedDiskElementDescriptor.java
    commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SortedPreferentialArray.java
    commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/utils/struct/SortedPrefArrayUnitTest.java

Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/indexed/IndexedDiskElementDescriptor.java
URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/indexed/IndexedDiskElementDescriptor.java?rev=1764695&r1=1764694&r2=1764695&view=diff
==============================================================================
--- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/indexed/IndexedDiskElementDescriptor.java (original)
+++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/indexed/IndexedDiskElementDescriptor.java Thu Oct 13 14:25:04 2016
@@ -78,16 +78,21 @@ public class IndexedDiskElementDescripto
     @Override
     public boolean equals(Object o)
     {
-        if (o instanceof IndexedDiskElementDescriptor)
+    	if (o == null)
+    	{
+    		return false;
+    	}
+    	else if (o instanceof IndexedDiskElementDescriptor)
         {
-            return compareTo((IndexedDiskElementDescriptor) o) == 0;
+    		IndexedDiskElementDescriptor ided = (IndexedDiskElementDescriptor)o;
+            return pos == ided.pos && len == ided.len;
         }
 
         return false;
     }
 
     /**
-     * Compares based on length.
+     * Compares based on length, then on pos descending.
      * <p>
      * @param o Object
      * @return int
@@ -100,19 +105,28 @@ public class IndexedDiskElementDescripto
             return 1;
         }
 
-        int oLen = o.len;
-        if ( oLen == len )
+        if ( o.len == len )
         {
-            return 0;
+        	if ( o.pos == pos )
+        	{
+        		return 0;
+        	}
+        	else if ( o.pos < pos )
+        	{
+        		return -1;
+        	}
+        	else
+        	{
+        		return 1;
+        	}
         }
-        else if ( oLen > len )
+        else if ( o.len > len )
         {
             return -1;
         }
-        else if ( oLen < len )
+        else
         {
             return 1;
         }
-        return 0;
     }
 }

Modified: commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SortedPreferentialArray.java
URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SortedPreferentialArray.java?rev=1764695&r1=1764694&r2=1764695&view=diff
==============================================================================
--- commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SortedPreferentialArray.java (original)
+++ commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SortedPreferentialArray.java Thu Oct 13 14:25:04 2016
@@ -1,5 +1,7 @@
 package org.apache.commons.jcs.utils.struct;
 
+import java.util.concurrent.ConcurrentSkipListSet;
+
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
@@ -25,8 +27,6 @@ import org.apache.commons.logging.LogFac
 /**
  * This maintains a sorted array with a preferential replacement policy when full.
  * <p>
- * Insertion time is n, search is log(n)
- * <p>
  * Clients must manage thread safety on previous version. I synchronized the public methods to add
  * easy thread safety. I synchronized all public methods that make modifications.
  */
@@ -41,14 +41,11 @@ public class SortedPreferentialArray<T e
     /** maximum number allowed */
     private int maxSize = 0;
 
-    /** The currency number */
+    /** The current number */
     private int curSize = 0;
 
     /** The primary array */
-    private final T[] array;
-
-    /** the number that have been inserted. */
-    private int insertCnt = 0;
+    private final ConcurrentSkipListSet<T> array;
 
     /**
      * Construct the array with the maximum size.
@@ -58,9 +55,7 @@ public class SortedPreferentialArray<T e
     public SortedPreferentialArray( int maxSize )
     {
         this.maxSize = maxSize;
-        @SuppressWarnings("unchecked") // No generic arrays in java
-        T[] ts = (T[]) new Comparable<?>[maxSize];
-        array = ts;
+        array = new ConcurrentSkipListSet<T>();
     }
 
     /**
@@ -79,38 +74,37 @@ public class SortedPreferentialArray<T e
         if ( curSize < maxSize )
         {
             insert( obj );
-            return;
         }
-        if ( preferLarge )
+        else if ( preferLarge )
         {
             // insert if obj is larger than the smallest
-            T sma = getSmallest();
-            if ( obj.compareTo( sma ) > 0 )
+            if ( obj.compareTo( array.first() ) > 0 )
             {
                 insert( obj );
-                return;
             }
             // obj is less than or equal to the smallest.
-            if ( log.isDebugEnabled() )
+            else if ( log.isDebugEnabled() )
             {
                 log.debug( "New object is smaller than or equal to the smallest" );
             }
-            return;
         }
-        // Not preferLarge
-        T lar = getLargest();
-        // insert if obj is smaller than the largest
-        int diff = obj.compareTo( lar );
-        if ( diff > 0 || diff == 0 )
+        else
         {
-            if ( log.isDebugEnabled() )
-            {
-                log.debug( "New object is larger than or equal to the largest" );
-            }
-            return;
+	        // Not preferLarge
+	        // insert if obj is smaller than the largest
+	        if ( obj.compareTo( array.last() ) >= 0)
+	        {
+	            if ( log.isDebugEnabled() )
+	            {
+	                log.debug( "New object is larger than or equal to the largest" );
+	            }
+	        }
+	        else
+	        {
+		        // obj is less than the largest.
+		        insert( obj );
+	        }
         }
-        // obj is less than the largest.
-        insert( obj );
     }
 
     /**
@@ -118,9 +112,9 @@ public class SortedPreferentialArray<T e
      * <p>
      * @return Comparable
      */
-    public synchronized T getLargest()
+    protected T getLargest()
     {
-        return array[curSize - 1];
+        return array.last();
     }
 
     /**
@@ -128,11 +122,12 @@ public class SortedPreferentialArray<T e
      * <p>
      * @return Comparable
      */
-    public synchronized T getSmallest()
+    protected T getSmallest()
     {
-        return array[0];
+        return array.first();
     }
 
+    
     /**
      * Insert looks for the nearest largest. It then determines which way to shuffle depending on
      * the preference.
@@ -141,117 +136,21 @@ public class SortedPreferentialArray<T e
      */
     private void insert(T obj)
     {
-        try
-        {
-            int nLar = findNearestLargerEqualOrLastPosition( obj );
-            if ( log.isDebugEnabled() )
-            {
-                log.debug( "nLar = " + nLar + " obj = " + obj );
-            }
-
-            if ( nLar == curSize )
-            {
-                // this next check should be unnecessary
-                // findNearestLargerPosition should only return the curSize if
-                // there is
-                // room left. Check to be safe
-                if ( curSize < maxSize )
-                {
-                    array[nLar] = obj;
-                    curSize++;
-                    if ( log.isDebugEnabled() )
-                    {
-                        log.debug( this.dumpArray() );
-                    }
-                    if ( log.isDebugEnabled() )
-                    {
-                        log.debug( "Inserted object at the end of the array" );
-                    }
-                    return;
-                } // end if not full
-            }
-
-            boolean isFull = false;
-            if ( curSize == maxSize )
+    	if (array.add(obj))
+    	{
+            if ( curSize < maxSize )
             {
-                isFull = true;
+            	curSize++;
             }
-
-            // The array is full, we must replace
-            // remove smallest or largest to determine whether to
-            // shuffle left or right to insert
-            if ( preferLarge )
+            else if (preferLarge)
             {
-                if ( isFull )
-                {
-                    // is full, prefer larger, remove smallest by shifting left
-                    int pnt = nLar - 1; // set iteration stop point
-                    for ( int i = 0; i < pnt; i++ )
-                    {
-                        array[i] = array[i + 1];
-                    }
-                    // use nLar-1 for insertion point
-                    array[nLar - 1] = obj;
-                    if ( log.isDebugEnabled() )
-                    {
-                        log.debug( "Inserted object at " + ( nLar - 1 ) );
-                    }
-                }
-                else
-                {
-                    // not full, shift right from spot
-                    int pnt = nLar; // set iteration stop point
-                    for ( int i = curSize; i > pnt; i-- )
-                    {
-                        array[i] = array[i - 1];
-                    }
-                    // use nLar-1 for insertion point
-                    array[nLar] = obj;
-                    curSize++;
-                    if ( log.isDebugEnabled() )
-                    {
-                        log.debug( "Inserted object at " + ( nLar ) );
-                    }
-                }
+            	array.pollFirst();
             }
             else
             {
-                // prefer smaller, remove largest by shifting right
-                // use nLar for insertion point
-                int pnt = nLar + 1;
-                if ( !isFull )
-                {
-                    pnt = nLar;
-                }
-                for ( int i = curSize; i > pnt; i-- )
-                {
-                    array[i] = array[i - 1];
-                }
-                array[nLar] = obj;
-                if ( log.isDebugEnabled() )
-                {
-                    log.debug( "Inserted object at " + nLar );
-                }
-            }
-
-            if ( log.isDebugEnabled() )
-            {
-                log.debug( this.dumpArray() );
-            }
-        }
-        catch ( Exception e )
-        {
-            log.error( "Insertion problem" + this.dumpArray(), e );
-        }
-
-        insertCnt++;
-        if ( insertCnt % 100 == 0 )
-        {
-            if ( log.isDebugEnabled() )
-            {
-                log.debug( this.dumpArray() );
+            	array.pollLast();
             }
-        }
+    	}
     }
 
     /**
@@ -265,47 +164,27 @@ public class SortedPreferentialArray<T e
     }
 
     /**
-     * Returns and removes the nearer larger or equal object from the aray.
+     * Returns and removes the nearer larger or equal object from the array.
      * <p>
      * @param obj Comparable
      * @return Comparable, null if arg is null or none was found.
      */
     public synchronized T takeNearestLargerOrEqual( T obj )
     {
-        if ( obj == null )
-        {
-            return null;
-        }
-
-        T retVal = null;
-        try
-        {
-            int pos = findNearestOccupiedLargerOrEqualPosition( obj );
-            if ( pos == -1 )
-            {
-                return null;
-            }
-
-            try
-            {
-                retVal = array[pos];
-                remove( pos );
-            }
-            catch ( Exception e )
-            {
-                log.error( "Problem removing from array. pos [" + pos + "] " + obj, e );
-            }
-
-            if ( log.isDebugEnabled() )
-            {
-                log.debug( "obj = " + obj + " || retVal = " + retVal );
-            }
-        }
-        catch ( Exception e )
-        {
-            log.error( "Take problem" + this.dumpArray(), e );
-        }
-        return retVal;
+    	if (obj == null)
+    	{
+    		return null;
+    	}
+    	
+    	T t = array.ceiling(obj);
+    	
+    	if (t != null)
+    	{
+	    	array.remove(t);
+	    	curSize--;
+    	}
+    	
+    	return t;
     }
 
     /**
@@ -317,296 +196,14 @@ public class SortedPreferentialArray<T e
     {
         return this.curSize;
     }
-
-    /**
-     * This determines the position in the array that is occupied by an object that is larger or
-     * equal to the argument. If none exists, -1 is returned.
-     * <p>
-     * @param obj Object
-     * @return Object
-     */
-    private int findNearestOccupiedLargerOrEqualPosition(T obj)
-    {
-        if ( curSize == 0 )
-        {
-            // nothing in the array
-            return -1;
-        }
-
-        // this gives us an insert position.
-        int pos = findNearestLargerEqualOrLastPosition( obj );
-
-        // see if the previous will do to handle the empty insert spot position
-        if ( pos == curSize )
-        { // && curSize < maxSize ) {
-            // pos will be > 0 if it equals curSize, we check for this above.
-            if ( obj.compareTo(array[pos - 1] ) <= 0 )
-            {
-                pos = pos - 1;
-            }
-            else
-            {
-                pos = -1;
-            }
-        }
-        else
-        {
-            // the find nearest, returns the last, since it is used by insertion.
-            if ( obj.compareTo(array[pos] ) > 0 )
-            {
-                return -1;
-            }
-        }
-
-        return pos;
-    }
-
+    
     /**
-     * This method determines the position where an insert should take place for a given object.
-     * With some additional checking, this can also be used to find an object matching or greater
-     * than the argument.
-     * <p>
-     * If the array is not full and the current object is larger than all the rest the first open
-     * slot at the end will be returned.
-     * <p>
-     * NOTE: If the object is larger than the largest and it is full, it will return the last position.
-     * <p>
-     * If the array is empty, the first spot is returned.
-     * <p>
-     * If the object is smaller than all the rests, the first position is returned. The caller must
-     * decide what to do given the preference.
-     * <p>
-     * Returns the position of the object nearest to or equal to the larger object.
-     * <p>
-     * If you want to find the takePosition, you have to calculate it.
-     * findNearestOccupiedLargerOrEqualPosition will calculate this for you.
-     * <p>
-     * @param obj Comparable
-     * @return int
+     * Get the underlying array (for testing)
+     * 
+     * @return a copy of the array
      */
-    private int findNearestLargerEqualOrLastPosition(T obj)
+	protected Object[] getArray()
     {
-        // do nothing if a null was passed in
-        if ( obj == null )
-        {
-            return -1;
-        }
-
-        // return the first spot if the array is empty
-        if ( curSize <= 0 )
-        {
-            return 0;
-        }
-
-        // mark the numer to be returned, the greaterPos as unset
-        int greaterPos = -1;
-        // prepare for a binary search
-        int curPos = ( curSize - 1 ) / 2;
-        int prevPos = -1;
-
-        try
-        {
-            // set the loop exit flag to false
-            boolean done = false;
-
-            // check the ends
-            // return insert position 0 if obj is smaller
-            // than the smallest. the caller can determine what to
-            // do with this, depending on the preference setting
-            if ( obj.compareTo( getSmallest() ) <= 0 )
-            {
-                // LESS THAN OR EQUAL TO SMALLEST
-                if ( log.isDebugEnabled() )
-                {
-                    log.debug( obj + " is smaller than or equal to " + getSmallest() );
-                }
-                greaterPos = 0;
-                done = true;
-                // return greaterPos;
-            }
-            else
-            {
-                // GREATER THAN SMALLEST
-                if ( log.isDebugEnabled() )
-                {
-                    log.debug( obj + " is bigger than " + getSmallest() );
-                }
-
-                // return the largest position if obj is larger
-                // than the largest. the caller can determine what to
-                // do with this, depending on the preference setting
-                if ( obj.compareTo( getLargest() ) >= 0 )
-                {
-                    if ( curSize == maxSize )
-                    {
-                        // there is no room left in the array, return the last
-                        // spot
-                        greaterPos = curSize - 1;
-                        done = true;
-                    }
-                    else
-                    {
-                        // there is room left in the array
-                        greaterPos = curSize;
-                        done = true;
-                    }
-                }
-                else
-                {
-                    // the obj is less than or equal to the largest, so we know that the
-                    // last item is larger or equal
-                    greaterPos = curSize - 1;
-                }
-            }
-
-            // /////////////////////////////////////////////////////////////////////
-            // begin binary search for insertion spot
-            while ( !done )
-            {
-                if ( log.isDebugEnabled() )
-                {
-                    log.debug( "\n curPos = " + curPos + "; greaterPos = " + greaterPos + "; prevpos = " + prevPos );
-                }
-
-                // get out of loop if we have come to the end or passed it
-                if ( curPos == prevPos || curPos >= curSize )
-                {
-                    done = true;
-                    break;
-                }
-                else
-
-                // EQUAL TO
-                // object at current position is equal to the obj, use this,
-                // TODO could avoid some shuffling if I found a lower pos.
-                if (array[curPos].compareTo( obj ) == 0 )
-                {
-                    if ( log.isDebugEnabled() )
-                    {
-                        log.debug( array[curPos] + " is equal to " + obj );
-                    }
-                    greaterPos = curPos;
-                    done = true;
-                    break;
-                }
-                else
-
-                // GREATER THAN
-                // array object at current position is greater than the obj, go
-                // left
-                if (array[curPos].compareTo( obj ) > 0 )
-                {
-                    if ( log.isDebugEnabled() )
-                    {
-                        log.debug( array[curPos] + " is greater than " + obj );
-                    }
-                    // set the smallest greater equal to the current position
-                    greaterPos = curPos;
-                    // set the current position to
-                    // set the previous position to the current position
-                    // We could have an integer overflow, but this array could
-                    // never get that large.
-                    int newPos = Math.min( curPos, ( curPos + prevPos ) / 2 );
-                    prevPos = curPos;
-                    curPos = newPos;
-                }
-                else
-
-                // LESS THAN
-                // the object at the current position is smaller, go right
-                if (array[curPos].compareTo( obj ) < 0 )
-                {
-                    if ( log.isDebugEnabled() )
-                    {
-                        log.debug( array[curPos] + " is less than " + obj );
-                    }
-                    if ( ( greaterPos != -1 ) && greaterPos - curPos < 0 )
-                    {
-                        done = true;
-                        break; // return greaterPos;
-                    }
-                    else
-                    {
-                        int newPos = 0;
-                        if ( prevPos > curPos )
-                        {
-                            newPos = Math.min( ( curPos + prevPos ) / 2, curSize );
-                        }
-                        else if ( prevPos == -1 )
-                        {
-                            newPos = Math.min( ( curSize + curPos ) / 2, curSize );
-                        }
-                        prevPos = curPos;
-                        curPos = newPos;
-                    }
-                }
-            } // end while
-            // /////////////////////////////////////////////////////////////////////
-
-            if ( log.isDebugEnabled() )
-            {
-                log.debug( "Greater Position is [" + greaterPos + "]" + " array[greaterPos] [" + array[greaterPos]
-                    + "]" );
-            }
-        }
-        catch ( Exception e )
-        {
-            log.error( "\n curPos = " + curPos + "; greaterPos = " + greaterPos + "; prevpos = " + prevPos + " "
-                + this.dumpArray(), e );
-        }
-
-        return greaterPos;
-    }
-
-    /**
-     * Removes the item from the array at the specified position. The remaining items to the right
-     * are shifted left.
-     * <p>
-     * @param position int
-     * @throw IndexOutOfBoundsException if position is out of range.
-     */
-    private void remove( int position )
-    {
-        if ( position >= curSize || position < 0 )
-        {
-            throw new IndexOutOfBoundsException( "position=" + position + " must be less than curSize=" + curSize );
-        }
-        curSize--;
-
-        if ( position < curSize )
-        {
-            try
-            {
-                System.arraycopy( array, position + 1, array, position, ( curSize - position ) );
-            }
-            catch ( IndexOutOfBoundsException ibe )
-            {
-                // throw this, log details for debugging. This shouldn't happen.
-                log.warn( "Caught index out of bounds exception. "
-                    + "called 'System.arraycopy( array, position + 1, array, position, (curSize - position) );'  "
-                    + "array.lengh [" + array.length + "] position [" + position + "] curSize [" + curSize + "]" );
-                throw ibe;
-            }
-        }
-    }
-
-    /**
-     * Debugging method to return a human readable display of array data.
-     * <p>
-     * @return String representation of the contents.
-     */
-    protected synchronized String dumpArray()
-    {
-        StringBuilder buf = new StringBuilder();
-        buf.append( "\n ---------------------------" );
-        buf.append( "\n curSize = " + curSize );
-        buf.append( "\n array.length = " + array.length );
-        buf.append( "\n ---------------------------" );
-        buf.append( "\n Dump:" );
-        for ( int i = 0; i < curSize; i++ )
-        {
-            buf.append( "\n " + i + "=" + array[i] );
-        }
-        return buf.toString();
+    	return array.toArray();
     }
 }

Modified: commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/utils/struct/SortedPrefArrayUnitTest.java
URL: http://svn.apache.org/viewvc/commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/utils/struct/SortedPrefArrayUnitTest.java?rev=1764695&r1=1764694&r2=1764695&view=diff
==============================================================================
--- commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/utils/struct/SortedPrefArrayUnitTest.java (original)
+++ commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/utils/struct/SortedPrefArrayUnitTest.java Thu Oct 13 14:25:04 2016
@@ -1,5 +1,7 @@
 package org.apache.commons.jcs.utils.struct;
 
+import java.util.Arrays;
+
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
@@ -88,14 +90,14 @@ public class SortedPrefArrayUnitTest
         for ( int i = 0; i < elem.length; i++ )
         {
             array.add( elem[i] );
-            // System.out.println( array.dumpArray() );
+            // System.out.println( Arrays.toString(array.getArray()) );
         }
 
         assertEquals( "Size was not as expected.", maxSize, array.size() );
 
         // this is a fragile test, since it relies on a hardcoded array
         String smallest = array.getSmallest();
-        assertEquals( "smallest should be 08", "08", smallest );
+        assertEquals( "smallest should be 07", "07", smallest );
 
         String largest = array.getLargest();
         assertEquals( "Largest should be 96", "96", largest );
@@ -105,7 +107,7 @@ public class SortedPrefArrayUnitTest
         assertEquals( "Taken should be 96", "96", taken );
         assertEquals( "Size was not as expected.", ( maxSize - 1 ), array.size() );
 
-        // System.out.println( array.dumpArray() );
+        // System.out.println( Arrays.toString(array.getArray()) );
     }
 
     /**
@@ -176,7 +178,7 @@ public class SortedPrefArrayUnitTest
         {
             array.add( elem[i] );
         }
-        // System.out.println( array.dumpArray() );
+        // System.out.println( Arrays.toString(array.getArray()) );
 
         assertEquals( "Size was not as expected.", maxSize, array.size() );
 
@@ -226,7 +228,7 @@ public class SortedPrefArrayUnitTest
         array.setPreferLarge( true );
 
         array.add( "10" );
-        // System.out.println( array.dumpArray() );
+        // System.out.println( Arrays.toString(array.getArray()) );
 
         try
         {
@@ -304,7 +306,7 @@ public class SortedPrefArrayUnitTest
         for ( int i = 0; i < elem.length; i++ )
         {
             array.add( elem[i] );
-            // System.out.println( array.dumpArray() );
+            // System.out.println( Arrays.toString(array.getArray()) );
         }
 
         assertEquals( "Size was not as expected.", maxSize, array.size() );
@@ -321,7 +323,7 @@ public class SortedPrefArrayUnitTest
         assertEquals( "Taken is not as expected", "09", taken );
         assertEquals( "Size was not as expected.", ( maxSize - 1 ), array.size() );
 
-        // System.out.println( "testTakeLastItem" + array.dumpArray() );
+        // System.out.println( "testTakeLastItem" + Arrays.toString(array.getArray()) );
     }
 
     /**
@@ -343,7 +345,7 @@ public class SortedPrefArrayUnitTest
         for ( int i = 0; i < elem.length; i++ )
         {
             array.add( elem[i] );
-            // System.out.println( array.dumpArray() );
+            // System.out.println( Arrays.toString(array.getArray()) );
         }
 
         assertEquals( "Size was not as expected.", maxSize, array.size() );
@@ -358,9 +360,9 @@ public class SortedPrefArrayUnitTest
         // this should take 96;
         String taken = array.takeNearestLargerOrEqual( "09" );
         assertEquals( "Taken is not as expected", "09", taken );
-        assertEquals( "Size was not as expected. " + array.dumpArray(), ( maxSize - 1 ), array.size() );
+        assertEquals( "Size was not as expected. " + Arrays.toString(array.getArray()), ( maxSize - 1 ), array.size() );
 
-        // System.out.println( "testTakeEveryLastItem" + array.dumpArray() );
+        // System.out.println( "testTakeEveryLastItem" + Arrays.toString(array.getArray()) );
 
         // take the rest
         // take more than the max in a reverse order
@@ -368,9 +370,9 @@ public class SortedPrefArrayUnitTest
         {
             array.takeNearestLargerOrEqual( elem[i] );
         }
-        // System.out.println( "testTakeEveryLastItem" + array.dumpArray() );
+        // System.out.println( "testTakeEveryLastItem" + Arrays.toString(array.getArray()) );
 
-        assertEquals( "There should nothing left. " + array.dumpArray(), 0, array.size() );
+        assertEquals( "There should nothing left. " + Arrays.toString(array.getArray()), 0, array.size() );
     }
 
     /**
@@ -389,12 +391,12 @@ public class SortedPrefArrayUnitTest
         for ( int i = 0; i < elem.length; i++ )
         {
             array.add( elem[i] );
-            // System.out.println( array.dumpArray() );
+            // System.out.println( Arrays.toString(array.getArray()) );
         }
 
         // DO WORK
         Comparable<String> taken = array.takeNearestLargerOrEqual( "04" );
-//        System.out.println( "testTakeLargerThanGreatest" + array.dumpArray() );
+//        System.out.println( "testTakeLargerThanGreatest" + Arrays.toString(array.getArray()) );
 
         assertNull( "We should have nothing since the largest element was smaller than what we asked for. "
             + " Instead we got " + taken, taken );
@@ -416,12 +418,12 @@ public class SortedPrefArrayUnitTest
         for ( int i = 0; i < elem.length; i++ )
         {
             array.add( elem[i] );
-            // System.out.println( array.dumpArray() );
+            // System.out.println( Arrays.toString(array.getArray()) );
         }
 
         // DO WORK
         Comparable<String> taken = array.takeNearestLargerOrEqual( "03" );
-        // System.out.println( "testEqualToGreatest_LastTwoSameSize" + array.dumpArray() );
+        // System.out.println( "testEqualToGreatest_LastTwoSameSize" + Arrays.toString(array.getArray()) );
 
         assertNotNull( "We should have something since the largest element was equal to what we asked for.", taken );
     }
@@ -442,12 +444,12 @@ public class SortedPrefArrayUnitTest
         for ( int i = 0; i < elem.length; i++ )
         {
             array.add( elem[i] );
-            // System.out.println( array.dumpArray() );
+            // System.out.println( Arrays.toString(array.getArray()) );
         }
 
         // DO WORK
         Comparable<String> taken = array.takeNearestLargerOrEqual( "03" );
-        // System.out.println( "testEqualToGreatest" + array.dumpArray() );
+        // System.out.println( "testEqualToGreatest" + Arrays.toString(array.getArray()) );
 
         assertNotNull( "We should have something since the largest element was equal to what we asked for.", taken );
     }



[jcs] Please review! was: svn commit: r1764695 - in /commons/proper/jcs/trunk/commons-jcs-core/src: main/java/org/apache/commons/jcs/auxiliary/disk/indexed/ main/java/org/apache/commons/jcs/utils/struct/ test/java/org/apache/commons/jcs/utils/struct/

Posted by Thomas Vandahl <tv...@apache.org>.
Hi folks,

On 13.10.16 16:25, tv@apache.org wrote:
> Author: tv
> Date: Thu Oct 13 14:25:04 2016
> New Revision: 1764695
> 
> URL: http://svn.apache.org/viewvc?rev=1764695&view=rev
> Log:
> Fix SortedPreferentialArray to actually do what was is supposed to do
> 
> Modified:
>     commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/auxiliary/disk/indexed/IndexedDiskElementDescriptor.java
>     commons/proper/jcs/trunk/commons-jcs-core/src/main/java/org/apache/commons/jcs/utils/struct/SortedPreferentialArray.java
>     commons/proper/jcs/trunk/commons-jcs-core/src/test/java/org/apache/commons/jcs/utils/struct/SortedPrefArrayUnitTest.java

When I looked into the tests of SortedPreferentialArray to see whether
this could be replaced with some JDK standard stuff, I found that it has
a number of severe problems. IMHO, it simply doesn't work. It doesn't
even sort correctly.

Now this class is used as a recycle bin in the IndexedDiskCache which,
to my knowledge, is a quite popular component within JCS.

Am I completely mistaken or is this functionality so unimportant that it
doesn't make any difference?

I replaced a lot of code with a reference to a ConcurrentSkipListSet and
managed to get all tests to pass again.

Could some kind soul please take a few cycles to review this commit and
the related code in IndexedDiskCache? I would like some help with
answering the following questions:

- The recycle bin contains elements of type
IndexedDiskElementDescriptor. Obviously, these elements were compared by
their size (len field) only, not their offset (pos field), so that
storage chunks of identical size are considered equal. Nevertheless,
this somehow seemed to work, even in the unit test where all elements
are of the same size. But why?

- The old implementation uses a fixed size array to store the elements.
If more elements were added, the smallest elements were discarded. Does
this actually make any sense?

- After my changes, the disk cache should recycle used storage chunks
more effectively. Can someone with a real-world application confirm this?

- Do I miss some other sophisticated feature in the code I removed?

Thank you in advance for any time you may be able to donate.

Bye, Thomas.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org