address Alex's 2nd round review comments
git-svn-id: https://asterixdb.googlecode.com/svn/branches/asterix_opentype@374 eaa15691-b419-025a-1212-ee371bd00084
diff --git a/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceDynamicTypeCastRule.java b/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceDynamicTypeCastRule.java
index 298bac0..26455f5 100644
--- a/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceDynamicTypeCastRule.java
+++ b/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceDynamicTypeCastRule.java
@@ -44,8 +44,8 @@
import edu.uci.ics.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
/**
- * dynamically cast a variable from its type to a specified required type, in a
- * recursive way it enables: 1. bag-based fields in a record, 2. bidirectional
+ * Dynamically cast a variable from its type to a specified required type, in a
+ * recursive way. It enables: 1. bag-based fields in a record, 2. bidirectional
* cast of a open field and a matched closed field, and 3. put in null fields
* when necessary
*
@@ -56,7 +56,8 @@
*
* However, if input record is a variable, then we don't know its exact field
* layout at compile time, for example, records confirming the same type can
- * different field ordering, different open part.
+ * different field ordering, different open part. That's why we need dynamic
+ * type casting.
*
* Note that as we can see in the example, the ordering of fields of a record is
* not required. Since the open/close part of a record has completely different
diff --git a/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceStaticTypeCastRule.java b/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceStaticTypeCastRule.java
index d6f2670..e14f53d 100644
--- a/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceStaticTypeCastRule.java
+++ b/asterix-algebra/src/main/java/edu/uci/ics/asterix/optimizer/rules/IntroduceStaticTypeCastRule.java
@@ -54,7 +54,7 @@
import edu.uci.ics.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
/**
- * statically cast a constant from its type produced by the originated
+ * Statically cast a constant from its type produced by the originated
* expression to its required type, in a recursive way it enables: 1. bag-based
* fields in a record, 2. bidirectional cast of a open field and a matched
* closed field, and 3. put in null fields when necessary. It should be fired
@@ -68,16 +68,16 @@
* vice versa.
*
* If the record is a constant, the situation that we are going into insert the
- * record into a dataset with a different type can be capatured at the compile
+ * record into a dataset with a different type can be captured at the compile
* time, and type cast is done in the rule.
*
* Implementation wise: first, we match the record's type and its target dataset
- * type to see if it is cast-able; second, if the types are cast-able, we embed the require type to the
- * original producer expression. If the types are not cast-able, we throw
- * compile time exceptions.
+ * type to see if it is "cast-able"; second, if the types are cast-able, we
+ * embed the require type to the original producer expression. If the types are
+ * not cast-able, we throw compile time exceptions.
*
- * Then, at runtime, the constructors know what to do by checking the required
- * output type.
+ * Then, at runtime (not in that rule), the constructors know what to do by
+ * checking the required output type.
*/
public class IntroduceStaticTypeCastRule implements IAlgebraicRewriteRule {
diff --git a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/ARecordPointable.java b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/ARecordPointable.java
index 5bfcb83..6634252 100644
--- a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/ARecordPointable.java
+++ b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/ARecordPointable.java
@@ -81,7 +81,6 @@
* private constructor, to prevent constructing it arbitrarily
*
* @param inputType
- * , the input type
*/
private ARecordPointable(ARecordType inputType) {
this.inputRecType = inputType;
diff --git a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/PointableAllocator.java b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/PointableAllocator.java
index 7675a2b..e6f7b5e 100644
--- a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/PointableAllocator.java
+++ b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/pointables/PointableAllocator.java
@@ -19,8 +19,8 @@
import edu.uci.ics.asterix.om.types.IAType;
import edu.uci.ics.asterix.runtime.pointables.base.DefaultOpenFieldType;
import edu.uci.ics.asterix.runtime.pointables.base.IVisitablePointable;
-import edu.uci.ics.asterix.runtime.util.container.IElementAllocator;
-import edu.uci.ics.asterix.runtime.util.container.ListElementAllocator;
+import edu.uci.ics.asterix.runtime.util.container.IObjectPool;
+import edu.uci.ics.asterix.runtime.util.container.ListObjectPool;
/**
* This class is the ONLY place to create IVisitablePointable object instances,
@@ -29,11 +29,11 @@
*/
public class PointableAllocator {
- private IElementAllocator<IVisitablePointable, IAType> flatValueAllocator = new ListElementAllocator<IVisitablePointable, IAType>(
+ private IObjectPool<IVisitablePointable, IAType> flatValueAllocator = new ListObjectPool<IVisitablePointable, IAType>(
AFlatValuePointable.FACTORY);
- private IElementAllocator<IVisitablePointable, IAType> recordValueAllocator = new ListElementAllocator<IVisitablePointable, IAType>(
+ private IObjectPool<IVisitablePointable, IAType> recordValueAllocator = new ListObjectPool<IVisitablePointable, IAType>(
ARecordPointable.FACTORY);
- private IElementAllocator<IVisitablePointable, IAType> listValueAllocator = new ListElementAllocator<IVisitablePointable, IAType>(
+ private IObjectPool<IVisitablePointable, IAType> listValueAllocator = new ListObjectPool<IVisitablePointable, IAType>(
AListPointable.FACTORY);
public IVisitablePointable allocateEmpty() {
diff --git a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/IElementAllocator.java b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/IObjectPool.java
similarity index 74%
rename from asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/IElementAllocator.java
rename to asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/IObjectPool.java
index 8172e56..9738985 100644
--- a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/IElementAllocator.java
+++ b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/IObjectPool.java
@@ -18,21 +18,19 @@
/**
* A reusable object pool interface
*/
-public interface IElementAllocator<E, T> {
+public interface IObjectPool<E, T> {
/**
* Give client an E instance
*
* @param arg
- * , the argument to create E
- * @return a E instance
+ * the argument to create E
+ * @return an E instance
*/
public E allocate(T arg);
/**
- * Clean all the used(assigned) instances in the pool. Then all instances in
- * the pool are marked as "unused" and can be returned to next bunch of
- * allocate call from a client
+ * Mark all instances in the pool as unused
*/
public void reset();
}
diff --git a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/ListElementAllocator.java b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/ListObjectPool.java
similarity index 76%
rename from asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/ListElementAllocator.java
rename to asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/ListObjectPool.java
index f54c7c8..a3c35be 100644
--- a/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/ListElementAllocator.java
+++ b/asterix-runtime/src/main/java/edu/uci/ics/asterix/runtime/util/container/ListObjectPool.java
@@ -21,17 +21,13 @@
import org.apache.hadoop.io.BooleanWritable;
/**
- * ListElementAllocator<E, T> is an element-reusable list or a element pool in
- * other words, however elements in the list should be exactly the same class,
- * this is forced by IElementFactory<E, T> factory as a parameter to the
- * constructor once a ListElementAllocator is constructed, it can only store
- * objects of the same class.
+ * Object pool backed by a list.
*
* The argument for creating E instances could be different. This class also
- * considers arguments in object reusing, e.g., reuse an E instances ONLY when
- * the construction argument is "equal".
+ * considers arguments in object reusing, e.g., it reuses an E instances ONLY
+ * when the construction argument is "equal".
*/
-public class ListElementAllocator<E, T> implements IElementAllocator<E, T> {
+public class ListObjectPool<E, T> implements IObjectPool<E, T> {
private IElementFactory<E, T> factory;
@@ -55,7 +51,7 @@
*/
private int minStartIndex = 0;
- public ListElementAllocator(IElementFactory<E, T> factory) {
+ public ListObjectPool(IElementFactory<E, T> factory) {
this.factory = factory;
}
@@ -64,16 +60,11 @@
boolean continuous = true;
for (int i = minStartIndex; i < pool.size(); i++) {
if (!usedBits.get(i).get()) {
- boolean match = false;
-
// the two cases where an element in the pool is a match
if ((arg == null && args.get(i) == null)
- || (arg != null && args.get(i) != null && arg.equals(args.get(i))))
- match = true;
-
- if (match) {
- // the element is not used and the arg is the same as input
- // arg
+ || (arg != null && args.get(i) != null && arg.equals(args.get(i)))) {
+ // the element is not used and the arg is the same as
+ // input arg
if (continuous) {
minStartIndex++;
}
@@ -92,7 +83,7 @@
}
}
- // if not find a reusable object, allocate a new element
+ // if we do not find a reusable object, allocate a new one
E element = factory.createElement(arg);
pool.add(element);
args.add(arg);