[SYSTEMML-1936] Column-indexing on main input in codegen row ops So far, the codegen framework removed invalid cplans with any indexing on the main input. On GLM binomial-probit this hugely affected performance because very large fused operators have been removed. For row-wise operations and column indexing, this is unnecessary. Hence, this patch generalizes the codegen compiler and runtime accordingly. For a scenario of GLM binomial-probit over a 100M x 10 dense input, w/ codegen, and 20/10 max iterations, this patch improved performance from 1,338s to 496s (2,722s w/o codegen). However, there is at least another factor of 4 possible.
Furthermore, this patch also improves the error handling for codegen java compilation errors to always show the generated source (otherwise the exceptions would be completely useless). Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/c5bec008 Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/c5bec008 Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/c5bec008 Branch: refs/heads/master Commit: c5bec00821f2c96ef0717b262ef260526c473eaa Parents: 0b04cb1 Author: Matthias Boehm <[email protected]> Authored: Tue Sep 26 11:51:23 2017 -0700 Committer: Matthias Boehm <[email protected]> Committed: Tue Sep 26 11:51:52 2017 -0700 ---------------------------------------------------------------------- .../sysml/hops/codegen/SpoofCompiler.java | 27 +++++++++--------- .../sysml/hops/codegen/cplan/CNodeBinary.java | 2 +- .../sysml/hops/codegen/cplan/CNodeTernary.java | 13 +++++++-- .../sysml/hops/codegen/cplan/CNodeUnary.java | 2 +- .../hops/codegen/template/TemplateUtils.java | 6 ++-- .../sysml/runtime/codegen/CodegenUtils.java | 8 +++++- .../sysml/runtime/codegen/SpoofOperator.java | 11 +++++++ .../functions/codegen/RowAggTmplTest.java | 18 +++++++++++- .../scripts/functions/codegen/rowAggPattern32.R | 30 ++++++++++++++++++++ .../functions/codegen/rowAggPattern32.dml | 25 ++++++++++++++++ 10 files changed, 119 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/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 af6c72b..1db2910 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java +++ b/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java @@ -691,7 +691,8 @@ public class SpoofCompiler //remove invalid plans with column indexing on main input if( tpl instanceof CNodeCell || tpl instanceof CNodeRow ) { CNodeData in1 = (CNodeData)tpl.getInput().get(0); - if( rHasLookupRC1(tpl.getOutput(), in1) || isLookupRC1(tpl.getOutput(), in1) ) { + boolean inclRC1 = !(tpl instanceof CNodeRow); + if( rHasLookupRC1(tpl.getOutput(), in1, inclRC1) || isLookupRC1(tpl.getOutput(), in1, inclRC1) ) { cplans2.remove(e.getKey()); if( LOG.isTraceEnabled() ) LOG.trace("Removed cplan due to invalid rc1 indexing on main input."); @@ -700,7 +701,7 @@ public class SpoofCompiler else if( tpl instanceof CNodeMultiAgg ) { CNodeData in1 = (CNodeData)tpl.getInput().get(0); for( CNode output : ((CNodeMultiAgg)tpl).getOutputs() ) - if( rHasLookupRC1(output, in1) || isLookupRC1(output, in1) ) { + if( rHasLookupRC1(output, in1, true) || isLookupRC1(output, in1, true) ) { cplans2.remove(e.getKey()); if( LOG.isTraceEnabled() ) LOG.trace("Removed cplan due to invalid rc1 indexing on main input."); @@ -712,7 +713,7 @@ public class SpoofCompiler if( tpl instanceof CNodeMultiAgg ) rFindAndRemoveLookupMultiAgg((CNodeMultiAgg)tpl, in1); else - rFindAndRemoveLookup(tpl.getOutput(), in1); + rFindAndRemoveLookup(tpl.getOutput(), in1, !(tpl instanceof CNodeRow)); //remove invalid row templates (e.g., unsatisfied blocksize constraint) if( tpl instanceof CNodeRow ) { @@ -770,44 +771,44 @@ public class SpoofCompiler private static void rFindAndRemoveLookupMultiAgg(CNodeMultiAgg node, CNodeData mainInput) { //process all outputs individually for( CNode output : node.getOutputs() ) - rFindAndRemoveLookup(output, mainInput); + rFindAndRemoveLookup(output, mainInput, true); //handle special case, of lookup being itself the output node for( int i=0; i < node.getOutputs().size(); i++) { CNode tmp = node.getOutputs().get(i); - if( TemplateUtils.isLookup(tmp) && tmp.getInput().get(0) instanceof CNodeData + if( TemplateUtils.isLookup(tmp, true) && tmp.getInput().get(0) instanceof CNodeData && ((CNodeData)tmp.getInput().get(0)).getHopID()==mainInput.getHopID() ) node.getOutputs().set(i, tmp.getInput().get(0)); } } - private static void rFindAndRemoveLookup(CNode node, CNodeData mainInput) { + private static void rFindAndRemoveLookup(CNode node, CNodeData mainInput, boolean includeRC1) { for( int i=0; i<node.getInput().size(); i++ ) { CNode tmp = node.getInput().get(i); - if( TemplateUtils.isLookup(tmp) && tmp.getInput().get(0) instanceof CNodeData + if( TemplateUtils.isLookup(tmp, includeRC1) && tmp.getInput().get(0) instanceof CNodeData && ((CNodeData)tmp.getInput().get(0)).getHopID()==mainInput.getHopID() ) { node.getInput().set(i, tmp.getInput().get(0)); } else - rFindAndRemoveLookup(tmp, mainInput); + rFindAndRemoveLookup(tmp, mainInput, includeRC1); } } - private static boolean rHasLookupRC1(CNode node, CNodeData mainInput) { + private static boolean rHasLookupRC1(CNode node, CNodeData mainInput, boolean includeRC1) { boolean ret = false; for( int i=0; i<node.getInput().size() && !ret; i++ ) { CNode tmp = node.getInput().get(i); - if( isLookupRC1(tmp, mainInput) ) + if( isLookupRC1(tmp, mainInput, includeRC1) ) ret = true; else - ret |= rHasLookupRC1(tmp, mainInput); + ret |= rHasLookupRC1(tmp, mainInput, includeRC1); } return ret; } - private static boolean isLookupRC1(CNode node, CNodeData mainInput) { - return (node instanceof CNodeTernary && (((CNodeTernary)node).getType()==TernaryType.LOOKUP_RC1 + private static boolean isLookupRC1(CNode node, CNodeData mainInput, boolean includeRC1) { + return (node instanceof CNodeTernary && ((((CNodeTernary)node).getType()==TernaryType.LOOKUP_RC1 && includeRC1) || ((CNodeTernary)node).getType()==TernaryType.LOOKUP_RVECT1 ) && node.getInput().get(0) instanceof CNodeData && ((CNodeData)node.getInput().get(0)).getHopID() == mainInput.getHopID()); http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java index bff044d..c2b5644 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java +++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeBinary.java @@ -270,7 +270,7 @@ public class CNodeBinary extends CNode //generate binary operation (use sparse template, if data input) boolean lsparse = sparse && (_inputs.get(0) instanceof CNodeData - && !_inputs.get(0).getVarname().startsWith("b") + && _inputs.get(0).getVarname().startsWith("a") && !_inputs.get(0).isLiteral()); boolean scalarInput = _inputs.get(0).getDataType().isScalar(); boolean scalarVector = (_inputs.get(0).getDataType().isScalar() http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeTernary.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeTernary.java b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeTernary.java index e9bb472..6e76816 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeTernary.java +++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeTernary.java @@ -54,7 +54,9 @@ public class CNodeTernary extends CNode return " double %TMP% = Double.isNaN(%IN1%) ? %IN3% : %IN1%;\n"; case LOOKUP_RC1: - return " double %TMP% = getValue(%IN1%, %IN2%, rowIndex, %IN3%-1);\n"; + return sparse ? + " double %TMP% = getValue(%IN1v%, %IN1i%, ai, alen, %IN3%-1);\n" : + " double %TMP% = getValue(%IN1%, %IN2%, rowIndex, %IN3%-1);\n"; case LOOKUP_RVECT1: return " double[] %TMP% = getVector(%IN1%, %IN2%, rowIndex, %IN3%-1);\n"; @@ -96,14 +98,19 @@ public class CNodeTernary extends CNode sb.append(_inputs.get(2).codegen(sparse)); //generate binary operation + boolean lsparse = sparse && (_inputs.get(0) instanceof CNodeData + && _inputs.get(0).getVarname().startsWith("a") + && !_inputs.get(0).isLiteral()); String var = createVarname(); - String tmp = _type.getTemplate(sparse); + String tmp = _type.getTemplate(lsparse); tmp = tmp.replace("%TMP%", var); for( int j=1; j<=3; j++ ) { String varj = _inputs.get(j-1).getVarname(); //replace sparse and dense inputs tmp = tmp.replace("%IN"+j+"v%", - varj+(varj.startsWith("b")?"":"vals") ); + varj+(varj.startsWith("a")?"vals":"") ); + tmp = tmp.replace("%IN"+j+"i%", + varj+(varj.startsWith("a")?"ix":"") ); tmp = tmp.replace("%IN"+j+"%", varj ); } sb.append(tmp); http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java index 343efb5..b3720dd 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java +++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeUnary.java @@ -198,7 +198,7 @@ public class CNodeUnary extends CNode //generate unary operation boolean lsparse = sparse && (_inputs.get(0) instanceof CNodeData - && !_inputs.get(0).getVarname().startsWith("b") + && _inputs.get(0).getVarname().startsWith("a") && !_inputs.get(0).isLiteral()); String var = createVarname(); String tmp = _type.getTemplate(lsparse); http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java index 95383e6..9d7baf9 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java +++ b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateUtils.java @@ -234,9 +234,9 @@ public class TemplateUtils throw new RuntimeException("Undefined outer product type for hop "+out.getHopID()); } - public static boolean isLookup(CNode node) { + public static boolean isLookup(CNode node, boolean includeRC1) { return isUnary(node, UnaryType.LOOKUP_R, UnaryType.LOOKUP_C, UnaryType.LOOKUP_RC) - || isTernary(node, TernaryType.LOOKUP_RC1); + || (includeRC1 && isTernary(node, TernaryType.LOOKUP_RC1)); } public static boolean isUnary(CNode node, UnaryType...types) { @@ -315,7 +315,7 @@ public class TemplateUtils public static boolean hasNoOperation(CNodeTpl tpl) { return tpl.getOutput() instanceof CNodeData - || isLookup(tpl.getOutput()); + || isLookup(tpl.getOutput(), true); } public static boolean hasOnlyDataNodeOrLookupInputs(CNode node) { http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/main/java/org/apache/sysml/runtime/codegen/CodegenUtils.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/codegen/CodegenUtils.java b/src/main/java/org/apache/sysml/runtime/codegen/CodegenUtils.java index b3316a2..ba5cebe 100644 --- a/src/main/java/org/apache/sysml/runtime/codegen/CodegenUtils.java +++ b/src/main/java/org/apache/sysml/runtime/codegen/CodegenUtils.java @@ -40,6 +40,8 @@ import javax.tools.StandardJavaFileManager; import javax.tools.ToolProvider; import org.apache.commons.io.IOUtils; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.sysml.api.DMLScript; import org.apache.sysml.hops.codegen.SpoofCompiler; import org.apache.sysml.hops.codegen.SpoofCompiler.CompilerType; @@ -51,6 +53,8 @@ import org.codehaus.janino.SimpleCompiler; public class CodegenUtils { + private static final Log LOG = LogFactory.getLog(CodegenUtils.class.getName()); + //cache to reuse compiled and loaded classes private static ConcurrentHashMap<String, Class<?>> _cache = new ConcurrentHashMap<String,Class<?>>(); @@ -167,6 +171,7 @@ public class CodegenUtils .loadClass(name); } catch(Exception ex) { + LOG.error("Failed to compile class "+name+": \n"+src); throw new DMLRuntimeException("Failed to compile class "+name+".", ex); } } @@ -232,7 +237,8 @@ public class CodegenUtils } } catch(Exception ex) { - throw new DMLRuntimeException(ex); + LOG.error("Failed to compile class "+name+": \n"+src); + throw new DMLRuntimeException("Failed to compile class "+name+".", ex); } } http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/main/java/org/apache/sysml/runtime/codegen/SpoofOperator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/codegen/SpoofOperator.java b/src/main/java/org/apache/sysml/runtime/codegen/SpoofOperator.java index fea51e3..156c5ff 100644 --- a/src/main/java/org/apache/sysml/runtime/codegen/SpoofOperator.java +++ b/src/main/java/org/apache/sysml/runtime/codegen/SpoofOperator.java @@ -21,6 +21,7 @@ package org.apache.sysml.runtime.codegen; import java.io.Serializable; import java.util.ArrayList; +import java.util.Arrays; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -177,6 +178,16 @@ public abstract class SpoofOperator implements Serializable return (data!=null) ? data[rowIndex*n+colIndex] : 0; } + protected static double getValue(double[] avals, int[] aix, int ai, int alen, double colIndex) { + int icolIndex = UtilFunctions.toInt(colIndex); + return getValue(avals, aix, ai, alen, icolIndex); + } + + protected static double getValue(double[] avals, int[] aix, int ai, int alen, int colIndex) { + int pos = Arrays.binarySearch(aix, ai, ai+alen, colIndex); + return (pos >= 0) ? avals[pos] : 0; + } + protected static double getValue(SideInput data, double rowIndex) { int irowIndex = UtilFunctions.toInt(rowIndex); return getValue(data, irowIndex); http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/test/java/org/apache/sysml/test/integration/functions/codegen/RowAggTmplTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/sysml/test/integration/functions/codegen/RowAggTmplTest.java b/src/test/java/org/apache/sysml/test/integration/functions/codegen/RowAggTmplTest.java index d4f87b3..78305e3 100644 --- a/src/test/java/org/apache/sysml/test/integration/functions/codegen/RowAggTmplTest.java +++ b/src/test/java/org/apache/sysml/test/integration/functions/codegen/RowAggTmplTest.java @@ -68,6 +68,7 @@ public class RowAggTmplTest extends AutomatedTestBase private static final String TEST_NAME29 = TEST_NAME+"29"; //sum(rowMins(X)) private static final String TEST_NAME30 = TEST_NAME+"30"; //Mlogreg inner core, multi-class private static final String TEST_NAME31 = TEST_NAME+"31"; //MLogreg - matrix-vector cbind 0s generalized + private static final String TEST_NAME32 = TEST_NAME+"32"; //X[, 1] - rowSums(X) private static final String TEST_DIR = "functions/codegen/"; private static final String TEST_CLASS_DIR = TEST_DIR + RowAggTmplTest.class.getSimpleName() + "/"; @@ -79,7 +80,7 @@ public class RowAggTmplTest extends AutomatedTestBase @Override public void setUp() { TestUtils.clearAssertionInformation(); - for(int i=1; i<=31; i++) + for(int i=1; i<=32; i++) addTestConfiguration( TEST_NAME+i, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME+i, new String[] { String.valueOf(i) }) ); } @@ -548,6 +549,21 @@ public class RowAggTmplTest extends AutomatedTestBase testCodegenIntegration( TEST_NAME31, false, ExecType.SPARK ); } + @Test + public void testCodegenRowAggRewrite32CP() { + testCodegenIntegration( TEST_NAME32, true, ExecType.CP ); + } + + @Test + public void testCodegenRowAgg32CP() { + testCodegenIntegration( TEST_NAME32, false, ExecType.CP ); + } + + @Test + public void testCodegenRowAgg32SP() { + testCodegenIntegration( TEST_NAME32, false, ExecType.SPARK ); + } + private void testCodegenIntegration( String testname, boolean rewrites, ExecType instType ) { boolean oldFlag = OptimizerUtils.ALLOW_ALGEBRAIC_SIMPLIFICATION; http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/test/scripts/functions/codegen/rowAggPattern32.R ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/codegen/rowAggPattern32.R b/src/test/scripts/functions/codegen/rowAggPattern32.R new file mode 100644 index 0000000..92a39c1 --- /dev/null +++ b/src/test/scripts/functions/codegen/rowAggPattern32.R @@ -0,0 +1,30 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +args<-commandArgs(TRUE) +options(digits=22) +library("Matrix") +library("matrixStats") + +X = matrix(seq(1,1500), 150, 10, byrow=TRUE); +R = X[,1] - rowSums(X); + +writeMM(as(R, "CsparseMatrix"), paste(args[2], "S", sep="")); http://git-wip-us.apache.org/repos/asf/systemml/blob/c5bec008/src/test/scripts/functions/codegen/rowAggPattern32.dml ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/codegen/rowAggPattern32.dml b/src/test/scripts/functions/codegen/rowAggPattern32.dml new file mode 100644 index 0000000..2482349 --- /dev/null +++ b/src/test/scripts/functions/codegen/rowAggPattern32.dml @@ -0,0 +1,25 @@ +#------------------------------------------------------------- +# +# 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. +# +#------------------------------------------------------------- + +X = matrix(seq(1,1500), 150, 10); +R = X[,1] - rowSums(X); + +write(R, $1)
