commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r1101516 - in /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl: GenericObjectPool.java PooledObject.java PooledObjectState.java
Date Wed, 11 May 2011 00:59:19 GMT
On 5/10/11 8:48 AM, markt@apache.org wrote:
> Author: markt
> Date: Tue May 10 15:48:22 2011
> New Revision: 1101516
>
> URL: http://svn.apache.org/viewvc?rev=1101516&view=rev
> Log:
> Move to using LinkedBlockingDeque for the queue of idle objects.
>
> Added:
>     commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java
  (with props)
>     commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObjectState.java
  (with props)
> Modified:
>     commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
>
> Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1101516&r1=1101515&r2=1101516&view=diff
> ==============================================================================
> --- commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
(original)
> +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
Tue May 10 15:48:22 2011
> @@ -19,16 +19,17 @@ package org.apache.commons.pool2.impl;
>  
>  import java.util.ArrayList;
>  import java.util.Collection;
> +import java.util.Iterator;
>  import java.util.LinkedList;
>  import java.util.List;
>  import java.util.NoSuchElementException;
>  import java.util.TimerTask;
> +import java.util.concurrent.LinkedBlockingDeque;
>  
>  import org.apache.commons.pool2.BaseObjectPool;
>  import org.apache.commons.pool2.ObjectPool;
>  import org.apache.commons.pool2.PoolUtils;
>  import org.apache.commons.pool2.PoolableObjectFactory;
> -import org.apache.commons.pool2.impl.GenericKeyedObjectPool.ObjectTimestampPair;
>  
>  /**
>   * A configurable {@link ObjectPool} implementation.
> @@ -595,7 +596,7 @@ public class GenericObjectPool<T> extend
>          _softMinEvictableIdleTimeMillis = softMinEvictableIdleTimeMillis;
>          _testWhileIdle = testWhileIdle;
>  
> -        _pool = new CursorableLinkedList<ObjectTimestampPair<T>>();
> +        _pool = new LinkedBlockingDeque<PooledObject<T>>();
>          startEvictor(_timeBetweenEvictionRunsMillis);
>      }
>  
> @@ -1147,7 +1148,7 @@ public class GenericObjectPool<T> extend
>                                          // Case 3: An object has been allocated
>                                          _numInternalProcessing--;
>                                          _numActive++;
> -                                        returnObject(latch.getPair().getValue());
> +                                        returnObject(latch.getPair().getObject());
>                                      }
>                                  }
>                                  if (doAllocate) {
> @@ -1182,7 +1183,7 @@ public class GenericObjectPool<T> extend
>              if(null == latch.getPair()) {
>                  try {
>                      T obj = _factory.makeObject();
> -                    latch.setPair(new ObjectTimestampPair<T>(obj));
> +                    latch.setPair(new PooledObject<T>(obj));
>                      newlyCreated = true;
>                  } finally {
>                      if (!newlyCreated) {
> @@ -1197,22 +1198,22 @@ public class GenericObjectPool<T> extend
>              }
>              // activate & validate the object
>              try {
> -                _factory.activateObject(latch.getPair().getValue());
> +                _factory.activateObject(latch.getPair().getObject());
>                  if(_testOnBorrow &&
> -                        !_factory.validateObject(latch.getPair().getValue())) {
> +                        !_factory.validateObject(latch.getPair().getObject())) {
>                      throw new Exception("ValidateObject failed");
>                  }
>                  synchronized(this) {
>                      _numInternalProcessing--;
>                      _numActive++;
>                  }
> -                return latch.getPair().getValue();
> +                return latch.getPair().getObject();
>              }
>              catch (Throwable e) {
>                  PoolUtils.checkRethrow(e);
>                  // object cannot be activated or is invalid
>                  try {
> -                    _factory.destroyObject(latch.getPair().getValue());
> +                    _factory.destroyObject(latch.getPair().getObject());
>                  } catch (Throwable e2) {
>                      PoolUtils.checkRethrow(e2);
>                      // cannot destroy broken object
> @@ -1310,11 +1311,11 @@ public class GenericObjectPool<T> extend
>       */
>      @Override
>      public void clear() {
> -        List<ObjectTimestampPair<T>> toDestroy = new ArrayList<ObjectTimestampPair<T>>();
> +        List<PooledObject<T>> toDestroy = new ArrayList<PooledObject<T>>();
>  
>          synchronized(this) {
>              toDestroy.addAll(_pool);
> -            _numInternalProcessing = _numInternalProcessing + _pool._size;
> +            _numInternalProcessing = _numInternalProcessing + _pool.size();
>              _pool.clear();
>          }
>          destroy(toDestroy, _factory);
> @@ -1329,10 +1330,10 @@ public class GenericObjectPool<T> extend
>       * @param c Collection of objects to destroy
>       * @param factory PoolableConnectionFactory used to destroy the objects
>       */
> -    private void destroy(Collection<ObjectTimestampPair<T>> c, PoolableObjectFactory<T>
factory) {
> -        for (ObjectTimestampPair<T> pair : c) {
> +    private void destroy(Collection<PooledObject<T>> c, PoolableObjectFactory<T>
factory) {
> +        for (PooledObject<T> pair : c) {
>              try {
> -                factory.destroyObject(pair.getValue());
> +                factory.destroyObject(pair.getObject());
>              } catch (Exception e) {
>                  // ignore error, keep destroying the rest
>              } finally {
> @@ -1441,9 +1442,9 @@ public class GenericObjectPool<T> extend
>                      // borrowObject always takes the first element from the queue,
>                      // so for LIFO, push on top, FIFO add to end
>                      if (_lifo) {
> -                        _pool.addFirst(new ObjectTimestampPair<T>(obj));
> +                        _pool.addFirst(new PooledObject<T>(obj));
>                      } else {
> -                        _pool.addLast(new ObjectTimestampPair<T>(obj));
> +                        _pool.addLast(new PooledObject<T>(obj));
>                      }
>                      if (decrementNumActive) {
>                          _numActive--;
> @@ -1508,7 +1509,7 @@ public class GenericObjectPool<T> extend
>      @Override
>      @Deprecated
>      public void setFactory(PoolableObjectFactory<T> factory) throws IllegalStateException
{
> -        List<ObjectTimestampPair<T>> toDestroy = new ArrayList<ObjectTimestampPair<T>>();
> +        List<PooledObject<T>> toDestroy = new ArrayList<PooledObject<T>>();
>          final PoolableObjectFactory<T> oldFactory = _factory;
>          synchronized (this) {
>              assertOpen();
> @@ -1516,7 +1517,7 @@ public class GenericObjectPool<T> extend
>                  throw new IllegalStateException("Objects are already active");
>              } else {
>                  toDestroy.addAll(_pool);
> -                _numInternalProcessing = _numInternalProcessing + _pool._size;
> +                _numInternalProcessing = _numInternalProcessing + _pool.size();
>                  _pool.clear();
>              }
>              _factory = factory;
> @@ -1539,82 +1540,89 @@ public class GenericObjectPool<T> extend
>       */
>      public void evict() throws Exception {
>          assertOpen();
> -        synchronized (this) {
> -            if(_pool.isEmpty()) {
> -                return;
> -            }
> -            if (null == _evictionCursor) {
> -                _evictionCursor = (_pool.cursor(_lifo ? _pool.size() : 0));
> -            }
> -        }
>  
> -        for (int i=0,m=getNumTests();i<m;i++) {
> -            final ObjectTimestampPair<T> pair;
> -            synchronized (this) {
> -                if ((_lifo && !_evictionCursor.hasPrevious()) ||
> -                        !_lifo && !_evictionCursor.hasNext()) {
> -                    _evictionCursor.close();
> -                    _evictionCursor = _pool.cursor(_lifo ? _pool.size() : 0);
> +        if (_pool.size() == 0) {
> +            return;
> +        }
> +        
> +        PooledObject<T> underTest = null;
> +        
> +        for (int i = 0, m = getNumTests(); i < m; i++) {
> +            if (_evictionIterator == null || !_evictionIterator.hasNext()) {
> +                if (getLifo()) {
> +                    _evictionIterator = _pool.descendingIterator();
> +                } else {
> +                    _evictionIterator = _pool.iterator();
>                  }
> -
> -                pair = _lifo ?
> -                        _evictionCursor.previous() :
> -                        _evictionCursor.next();
> -
> -                _evictionCursor.remove();
> -                _numInternalProcessing++;
> -            }
> -
> -            boolean removeObject = false;
> -            final long idleTimeMilis = System.currentTimeMillis() - pair.getTstamp();
> -            if ((getMinEvictableIdleTimeMillis() > 0) &&
> -                    (idleTimeMilis > getMinEvictableIdleTimeMillis())) {
> -                removeObject = true;
> -            } else if ((getSoftMinEvictableIdleTimeMillis() > 0) &&
> -                    (idleTimeMilis > getSoftMinEvictableIdleTimeMillis()) &&
> -                    ((getNumIdle() + 1)> getMinIdle())) { // +1 accounts for object
we are processing
> -                removeObject = true;
>              }
> -            if(getTestWhileIdle() && !removeObject) {
> -                boolean active = false;
> +            if (!_evictionIterator.hasNext()) {
> +                // Pool exhausted, nothing to do here
> +                return;
> +            } else {
>                  try {
> -                    _factory.activateObject(pair.getValue());
> -                    active = true;
> -                } catch(Exception e) {
> -                    removeObject=true;
> -                }
> -                if(active) {
> -                    if(!_factory.validateObject(pair.getValue())) {
> -                        removeObject=true;
> -                    } else {
> -                        try {
> -                            _factory.passivateObject(pair.getValue());
> -                        } catch(Exception e) {
> -                            removeObject=true;
> -                        }
> -                    }
> +                    underTest = _evictionIterator.next();
> +                } catch (NoSuchElementException nsee) {
> +                    // Object was borrowed in another thread
> +                    // Don't count this as an eviction test so reduce i;
> +                    i--;
> +                    _evictionIterator = null;
> +                    continue;
>                  }
>              }
>  
> -            if (removeObject) {
> -                try {
> -                    _factory.destroyObject(pair.getValue());
> -                } catch(Exception e) {
> -                    // ignored
> -                }
> -            }
> -            synchronized (this) {
> -                if(!removeObject) {
> -                    _evictionCursor.add(pair);
> -                    if (_lifo) {
> -                        // Skip over the element we just added back
> -                        _evictionCursor.previous();
> +            if (!underTest.startEvictionTest()) {
> +                // Object was borrowed in another thread
> +                // Don't count this as an eviction test so reduce i;
> +                i--;
> +                continue;
> +            }
> +            
> +            if (getMinEvictableIdleTimeMillis() > 0 &&
> +                        getMinEvictableIdleTimeMillis() <
> +                            underTest.getIdleTimeMillis() ||
> +                    (getSoftMinEvictableIdleTimeMillis() > 0 &&
> +                            getSoftMinEvictableIdleTimeMillis() <
> +                                underTest.getIdleTimeMillis() &&
> +                            getMinIdle() < _pool.size())) {
> +                destroy(underTest);
> +            } else {
> +                if (getTestWhileIdle()) {
> +                    boolean active = false;
> +                    try {
> +                        _factory.activateObject(underTest.getObject());
> +                        active = true;
> +                    } catch(Exception e) {
> +                        destroy(underTest);
>                      }
> +                    if(active) {
> +                        if(!_factory.validateObject(underTest.getObject())) {
> +                            destroy(underTest);
> +                        } else {
> +                            try {
> +                                _factory.passivateObject(underTest.getObject());
> +                            } catch(Exception e) {
> +                                destroy(underTest);
> +                            }
> +                        }
> +                    }
> +                }
> +                if (!underTest.endEvictionTest()) {
> +                    // TODO - May need to add code here once additional states
> +                    // are used
>                  }
> -                _numInternalProcessing--;
>              }
>          }
> -        allocate();
> +
> +        return;
> +    }
> +
> +    private void destroy(PooledObject<T> toDestory) {
> +        _pool.remove(toDestory);
> +        try {
> +            _factory.destroyObject(toDestory.getObject());
> +        } catch (Exception e) {
> +            // Ignore
> +        }
>      }
>  
>      /**
> @@ -1720,9 +1728,8 @@ public class GenericObjectPool<T> extend
>          buf.append("Active: ").append(getNumActive()).append("\n");
>          buf.append("Idle: ").append(getNumIdle()).append("\n");
>          buf.append("Idle Objects:\n");
> -        long time = System.currentTimeMillis();
> -        for (ObjectTimestampPair<T> pair  : _pool) {
> -            buf.append("\t").append(pair.getValue()).append("\t").append(time - pair.getTstamp()).append("\n");
           
> +        for (PooledObject<T> pair  : _pool) {
> +            buf.append("\t").append(pair.toString());            
>          }
>          return buf.toString();
>      }
> @@ -1845,7 +1852,7 @@ public class GenericObjectPool<T> extend
>      private final class Latch {
>  
>          /** object timestamp pair allocated to this latch */
> -        private ObjectTimestampPair<T> _pair;
> +        private PooledObject<T> _pair;
>  
>          /** Whether or not this latch may create an object instance */
>          private boolean _mayCreate = false;
> @@ -1854,7 +1861,7 @@ public class GenericObjectPool<T> extend
>           * Returns ObjectTimestampPair allocated to this latch
>           * @return ObjectTimestampPair allocated to this latch
>           */
> -        private synchronized ObjectTimestampPair<T> getPair() {
> +        private synchronized PooledObject<T> getPair() {
>              return _pair;
>          }
>          
> @@ -1862,7 +1869,7 @@ public class GenericObjectPool<T> extend
>           * Sets ObjectTimestampPair on this latch
>           * @param pair ObjectTimestampPair allocated to this latch
>           */
> -        private synchronized void setPair(ObjectTimestampPair<T> pair) {
> +        private synchronized void setPair(PooledObject<T> pair) {
>              _pair = pair;
>          }
>  
> @@ -2042,10 +2049,9 @@ public class GenericObjectPool<T> extend
>      private boolean _lifo = DEFAULT_LIFO;
>  
>      /** My pool. */
> -    private CursorableLinkedList<ObjectTimestampPair<T>> _pool = null;
> +    private LinkedBlockingDeque<PooledObject<T>> _pool = null;
>  
> -    /** Eviction cursor - keeps track of idle object evictor position */
> -    private CursorableLinkedList<ObjectTimestampPair<T>>.Cursor _evictionCursor
= null;
> +    private Iterator<PooledObject<T>> _evictionIterator = null;
>  
>      /** My {@link PoolableObjectFactory}. */
>      private PoolableObjectFactory<T> _factory;
>
> Added: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java?rev=1101516&view=auto
> ==============================================================================
> --- commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java
(added)
> +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java
Tue May 10 15:48:22 2011
> @@ -0,0 +1,79 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements.  See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License.  You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.commons.pool2.impl;
> +
> +/**
> + * This wrapper is used to track the additional information, such as state, for
> + * the pooled objects.
> + */
> +public class PooledObject<T> {
> +
> +    private T _object = null;
> +    private volatile PooledObjectState _state = PooledObjectState.IDLE;
> +    private long _lastActiveTime = System.currentTimeMillis();
> +
> +    public PooledObject(T object) {
> +        _object = object;
> +    }
> +    
> +    /**
> +     * Obtain the underlying object that is wrapped by this instance of
> +     * {@link PooledObject}.
> +     */
> +    public T getObject() {
> +        return _object;
> +    }
> +
> +    /**
> +     * Obtain the time in milliseconds since this object was last active.
> +     */
> +    public long getIdleTimeMillis() {
> +        return System.currentTimeMillis() - _lastActiveTime;
> +    }
> +
> +    /**
> +     * Provides a String form of the wrapper for debug purposes. The format is
> +     * not fixed and may change at any time.
> +     */
> +    @Override
> +    public String toString() {
> +        StringBuilder result = new StringBuilder();
> +        result.append("Object: ");
> +        result.append(_object.toString());
> +        result.append(", State: ");
> +        result.append(_state.toString());
> +        return result.toString();
> +    }

Could be I am not following the logic here, but seems to me that
better names below would be startMaintenance / endMaintenance

Phil

> +    public synchronized boolean startEvictionTest() {
> +        if (_state == PooledObjectState.IDLE) {
> +            _state = PooledObjectState.MAINTAIN_EVICTION;
> +            return true;
> +        }
> +        
> +        return false;
> +    }
> +
> +    public synchronized boolean endEvictionTest() {
> +        if (_state == PooledObjectState.MAINTAIN_EVICTION) {
> +            _state = PooledObjectState.IDLE;
> +            return true;
> +        }
> +        
> +        return false;
> +    }
> +}
>
> Propchange: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java
> ------------------------------------------------------------------------------
>     svn:eol-style = native
>
> Added: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObjectState.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObjectState.java?rev=1101516&view=auto
> ==============================================================================
> --- commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObjectState.java
(added)
> +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObjectState.java
Tue May 10 15:48:22 2011
> @@ -0,0 +1,54 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements.  See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License.  You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.commons.pool2.impl;
> +
> +/**
> + * Provides the possible states that a {@link PooledObject} may be in.
> + * <ul>
> + * <li>{@link #IDLE}: In queue, not in use.</li>
> + * <li>{@link #ALLOCATED}: In use.</li>
> + * <li>{@link #MAINTAIN_EVICTION}: In queue, currently being tested for possible
> + *     eviction.</li>
> + * <li>{@link #MAINTAIN_EVICTION_RETURN_TO_HEAD}: Not in queue, currently being
> + *     tested for possible eviction. An attempt to borrow the object was made
> + *     while being tested which removed it from the queue. It should be returned
> + *     to the head of the queue once testing completes.</li>
> + * <li>{@link #MAINTAIN_VALIDATION}: In queue, currently being validated.</li>
> + * <li>{@link #MAINTAIN_VALIDATION_PREALLOCATED}: Not in queue, currently being
> + *     validated. The object was borrowed while being validated which removed it
> + *     from the queue. It should be allocated once validation completes.</li>
> + * <li>{@link #MAINTAIN_VALIDATION_RETURN_TO_HEAD}: Not in queue, currently
> + *     being validated. An attempt to borrow the object was made while
> + *     previously being tested for eviction which removed it from the queue. It
> + *     should be returned to the head of the queue once validation completes.
> + *     </li>
> + * <li>{@link #INVALID}: Failed maintenance (e.g. eviction test or validation)
> + *     and will be / has been destroyed.</li>
> + * </ul>
> + * 
> + * TODO: Find shorter names for these states without loss of meaning.
> + */
> +public enum PooledObjectState {
> +    IDLE,
> +    ALLOCATED,
> +    MAINTAIN_EVICTION,
> +    MAINTAIN_EVICTION_RETURN_TO_HEAD,
> +    MAINTAIN_VALIDATION,
> +    MAINTAIN_VALIDATION_PREALLOCATED,
> +    MAINTAIN_VALIDATION_RETURN_TO_HEAD,
> +    INVALID
> +}
>
> Propchange: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObjectState.java
> ------------------------------------------------------------------------------
>     svn:eol-style = native
>
>
>


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


Mime
View raw message