You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemml.apache.org by mb...@apache.org on 2017/10/18 04:39:30 UTC
[1/2] systemml git commit: [SYSTEMML-1966] Fix codegen plan cache
false negatives, cleanups
Repository: systemml
Updated Branches:
refs/heads/master 259814e6c -> 5b8d62659
[SYSTEMML-1966] Fix codegen plan cache false negatives, cleanups
This patch fixes the codegen plan cache to prevent false negatives due
to different hash codes that are only based on different data nodes.
Furthermore, this also includes some smaller cleanups such as the
compilation of mult2 and pow2 in outer templates as well as the
exploration of valid fusion plans when using fusion heuristics.
Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/60ad522e
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/60ad522e
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/60ad522e
Branch: refs/heads/master
Commit: 60ad522eb4107e55cd9418cc8494208e6c36e28c
Parents: 259814e
Author: Matthias Boehm <mb...@gmail.com>
Authored: Tue Oct 17 20:16:33 2017 -0700
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Tue Oct 17 21:39:57 2017 -0700
----------------------------------------------------------------------
.../org/apache/sysml/hops/codegen/SpoofCompiler.java | 12 ++++++++----
.../apache/sysml/hops/codegen/cplan/CNodeData.java | 6 +++---
.../hops/codegen/template/CPlanCSERewriter.java | 2 +-
.../hops/codegen/template/TemplateOuterProduct.java | 15 ++++++++++-----
.../sysml/hops/codegen/template/TemplateRow.java | 9 ++++++---
.../sysml/runtime/matrix/data/LibMatrixMult.java | 6 +++---
6 files changed, 31 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java b/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java
index 5ff90fb..4af8540 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java
@@ -139,6 +139,10 @@ public class SpoofCompiler
return this == FUSE_ALL
|| this == FUSE_NO_REDUNDANCY;
}
+ public boolean isCostBased() {
+ return this == FUSE_COST_BASED_V2
+ || this == FUSE_COST_BASED;
+ }
}
public enum PlanCachePolicy {
@@ -399,14 +403,14 @@ public class SpoofCompiler
//explain debug output cplans or generated source code
if( LOG.isTraceEnabled() || DMLScript.EXPLAIN.isHopsType(recompile) ) {
- LOG.info("Codegen EXPLAIN (generated cplan for HopID: "
- + cplan.getKey() + ", line "+tmp.getValue().getBeginLine() + "):");
+ LOG.info("Codegen EXPLAIN (generated cplan for HopID: " + cplan.getKey() +
+ ", line "+tmp.getValue().getBeginLine() + ", hash="+tmp.getValue().hashCode()+"):");
LOG.info(tmp.getValue().getClassname()
+ Explain.explainCPlan(cplan.getValue().getValue()));
}
if( LOG.isTraceEnabled() || DMLScript.EXPLAIN.isRuntimeType(recompile) ) {
- LOG.info("Codegen EXPLAIN (generated code for HopID: "
- + cplan.getKey() + ", line "+tmp.getValue().getBeginLine() + "):");
+ LOG.info("Codegen EXPLAIN (generated code for HopID: " + cplan.getKey() +
+ ", line "+tmp.getValue().getBeginLine() + ", hash="+tmp.getValue().hashCode()+"):");
LOG.info(src);
}
http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
index ce343cf..653b50b 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java
@@ -106,9 +106,9 @@ public class CNodeData extends CNode
public boolean equals(Object o) {
return (o instanceof CNodeData
&& super.equals(o)
- && isLiteral() == ((CNodeData)o).isLiteral()
- && (isLiteral() || !_strictEquals) ?
+ && isLiteral() == ((CNode)o).isLiteral()
+ && ((isLiteral() || !_strictEquals) ?
_name.equals(((CNodeData)o)._name) :
- _hopID == ((CNodeData)o)._hopID);
+ _hopID == ((CNodeData)o)._hopID));
}
}
http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java b/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java
index 3d12cfe..318e30a 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java
@@ -56,7 +56,7 @@ public class CPlanCSERewriter
//step 3: reset data nodes to imprecise comparison
tpl.resetVisitStatusOutputs();
for( CNode out : outputs )
- rSetStrictDataNodeComparision(out, true);
+ rSetStrictDataNodeComparision(out, false);
tpl.resetVisitStatusOutputs();
return tpl;
http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java
index f3880b1..256f540 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java
@@ -174,6 +174,7 @@ public class TemplateOuterProduct extends TemplateBase {
}
else if(hop instanceof BinaryOp)
{
+ BinaryOp bop = (BinaryOp) hop;
CNode cdata1 = tmp.get(hop.getInput().get(0).getHopID());
CNode cdata2 = tmp.get(hop.getInput().get(1).getHopID());
String primitiveOpName = ((BinaryOp)hop).getOp().toString();
@@ -186,8 +187,12 @@ public class TemplateOuterProduct extends TemplateBase {
//add lookups if required
cdata1 = TemplateUtils.wrapLookupIfNecessary(cdata1, hop.getInput().get(0));
cdata2 = TemplateUtils.wrapLookupIfNecessary(cdata2, hop.getInput().get(1));
-
- out = new CNodeBinary(cdata1, cdata2, BinType.valueOf(primitiveOpName));
+ if( bop.getOp()==OpOp2.POW && cdata2.isLiteral() && cdata2.getVarname().equals("2") )
+ out = new CNodeUnary(cdata1, UnaryType.POW2);
+ else if( bop.getOp()==OpOp2.MULT && cdata2.isLiteral() && cdata2.getVarname().equals("2") )
+ out = new CNodeUnary(cdata1, UnaryType.MULT2);
+ else
+ out = new CNodeBinary(cdata1, cdata2, BinType.valueOf(primitiveOpName));
}
else if(hop instanceof AggBinaryOp)
{
@@ -213,14 +218,14 @@ public class TemplateOuterProduct extends TemplateBase {
//final left/right matrix mult, see close
else {
if( cdata1.getDataType().isScalar() )
- out = new CNodeBinary(cdata2, cdata1, BinType.VECT_MULT_ADD);
+ out = new CNodeBinary(cdata2, cdata1, BinType.VECT_MULT_ADD);
else
- out = new CNodeBinary(cdata1, cdata2, BinType.VECT_MULT_ADD);
+ out = new CNodeBinary(cdata1, cdata2, BinType.VECT_MULT_ADD);
}
}
else if( HopRewriteUtils.isTransposeOperation(hop) )
{
- out = tmp.get(hop.getInput().get(0).getHopID());
+ out = tmp.get(hop.getInput().get(0).getHopID());
}
else if( hop instanceof AggUnaryOp && ((AggUnaryOp)hop).getOp() == AggOp.SUM
&& ((AggUnaryOp)hop).getDirection() == Direction.RowCol )
http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java
index 64014da..0389983 100644
--- a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java
+++ b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java
@@ -33,6 +33,7 @@ import org.apache.sysml.hops.LiteralOp;
import org.apache.sysml.hops.ParameterizedBuiltinOp;
import org.apache.sysml.hops.TernaryOp;
import org.apache.sysml.hops.UnaryOp;
+import org.apache.sysml.hops.codegen.SpoofCompiler;
import org.apache.sysml.hops.codegen.cplan.CNode;
import org.apache.sysml.hops.codegen.cplan.CNodeBinary;
import org.apache.sysml.hops.codegen.cplan.CNodeBinary.BinType;
@@ -83,7 +84,8 @@ public class TemplateRow extends TemplateBase
&& hop.getInput().get(0).getDim1()>1 && hop.getInput().get(0).getDim2()>1)
|| (hop instanceof AggBinaryOp && hop.dimsKnown() && LibMatrixMult.isSkinnyRightHandSide(
hop.getInput().get(0).getDim1(), hop.getInput().get(0).getDim2(), //MM
- hop.getInput().get(1).getDim1(), hop.getInput().get(1).getDim2())
+ hop.getInput().get(1).getDim1(), hop.getInput().get(1).getDim2(),
+ SpoofCompiler.PLAN_SEL_POLICY.isCostBased())
&& hop.getInput().get(0).getDim1()>1 && hop.getInput().get(0).getDim2()>1
&& !HopRewriteUtils.isOuterProductLikeMM(hop))
|| (HopRewriteUtils.isTransposeOperation(hop) && hop.getParent().size()==1
@@ -152,8 +154,9 @@ public class TemplateRow extends TemplateBase
//check for fusable but not opening matrix multiply (vect_outer-mult)
Hop in1 = hop.getInput().get(0); //transpose
Hop in2 = hop.getInput().get(1);
- return LibMatrixMult.isSkinnyRightHandSide(in1.getDim2(), in1.getDim1(), hop.getDim1(), hop.getDim2())
- || LibMatrixMult.isSkinnyRightHandSide(in2.getDim1(), in2.getDim2(), hop.getDim2(), hop.getDim1());
+ boolean inclSizes = SpoofCompiler.PLAN_SEL_POLICY.isCostBased();
+ return LibMatrixMult.isSkinnyRightHandSide(in1.getDim2(), in1.getDim1(), hop.getDim1(), hop.getDim2(), inclSizes)
+ || LibMatrixMult.isSkinnyRightHandSide(in2.getDim1(), in2.getDim2(), hop.getDim2(), hop.getDim1(), inclSizes);
}
private static boolean isPartOfValidCumAggChain(Hop hop) {
http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
index eca26f6..fa4d667 100644
--- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
+++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java
@@ -3567,13 +3567,13 @@ public class LibMatrixMult
private static boolean checkPrepMatrixMultRightInput( MatrixBlock m1, MatrixBlock m2 ) {
//transpose if dense-dense, skinny rhs matrix (not vector), and memory guarded by output
return (LOW_LEVEL_OPTIMIZATION && !m1.sparse && !m2.sparse
- && isSkinnyRightHandSide(m1.rlen, m1.clen, m2.rlen, m2.clen));
+ && isSkinnyRightHandSide(m1.rlen, m1.clen, m2.rlen, m2.clen, true));
}
//note: public for use by codegen for consistency
- public static boolean isSkinnyRightHandSide(long m1rlen, long m1clen, long m2rlen, long m2clen) {
+ public static boolean isSkinnyRightHandSide(long m1rlen, long m1clen, long m2rlen, long m2clen, boolean inclCacheSize) {
return m1rlen > m2clen && m2rlen > m2clen && m2clen > 1
- && m2clen < 64 && 8*m2rlen*m2clen < L2_CACHESIZE;
+ && m2clen < 64 && (!inclCacheSize || 8*m2rlen*m2clen < L2_CACHESIZE);
}
public static boolean checkParColumnAgg(MatrixBlock m1, int k, boolean inclFLOPs) {
[2/2] systemml git commit: [MINOR] Fix missing warning on truncation
of matrix/frame toString
Posted by mb...@apache.org.
[MINOR] Fix missing warning on truncation of matrix/frame toString
Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/5b8d6265
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/5b8d6265
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/5b8d6265
Branch: refs/heads/master
Commit: 5b8d62659b2e5727bebcaf0d2681fc4ecd4ea85f
Parents: 60ad522
Author: Matthias Boehm <mb...@gmail.com>
Authored: Tue Oct 17 20:54:01 2017 -0700
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Tue Oct 17 21:39:58 2017 -0700
----------------------------------------------------------------------
.../cp/ParameterizedBuiltinCPInstruction.java | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/systemml/blob/5b8d6265/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
index e8a5f4a..f6532d7 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
@@ -26,6 +26,7 @@ import org.apache.sysml.lops.Lop;
import org.apache.sysml.parser.ParameterizedBuiltinFunctionExpression;
import org.apache.sysml.parser.Statement;
import org.apache.sysml.runtime.DMLRuntimeException;
+import org.apache.sysml.runtime.controlprogram.caching.CacheBlock;
import org.apache.sysml.runtime.controlprogram.caching.CacheableData;
import org.apache.sysml.runtime.controlprogram.caching.FrameObject;
import org.apache.sysml.runtime.controlprogram.caching.MatrixObject;
@@ -328,10 +329,12 @@ public class ParameterizedBuiltinCPInstruction extends ComputationCPInstruction
String out = null;
if( data instanceof MatrixObject ) {
MatrixBlock matrix = (MatrixBlock) data.acquireRead();
+ warnOnTrunction(matrix, rows, cols);
out = DataConverter.toString(matrix, sparse, separator, lineseparator, rows, cols, decimal);
}
else if( data instanceof FrameObject ) {
FrameBlock frame = (FrameBlock) data.acquireRead();
+ warnOnTrunction(frame, rows, cols);
out = DataConverter.toString(frame, sparse, separator, lineseparator, rows, cols, decimal);
}
else {
@@ -342,6 +345,17 @@ public class ParameterizedBuiltinCPInstruction extends ComputationCPInstruction
}
else {
throw new DMLRuntimeException("Unknown opcode : " + opcode);
- }
+ }
+ }
+
+ private void warnOnTrunction(CacheBlock data, int rows, int cols) {
+ //warn on truncation because users might not be aware and use toString for verification
+ if( (getParam("rows")==null && data.getNumRows()>rows)
+ || (getParam("cols")==null && data.getNumColumns()>cols) )
+ {
+ LOG.warn("Truncating "+data.getClass().getSimpleName()+" of size "
+ + data.getNumRows()+"x"+data.getNumColumns()+" to "+rows+"x"+cols+". "
+ + "Use toString(X, rows=..., cols=...) if necessary.");
+ }
}
}