Repository: systemml Updated Branches: refs/heads/master c7604ea26 -> 864bfc912
[HOTFIX][SYSTEMML-2095] Fix runtime integration parfor result variables This patch fixes the recent runtime integration of extended parfor result variables (from variable names to meta data handles), which led to wrong parfor optimizer decisions resulting in, for example, OOMs on sparse use cases. Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/864bfc91 Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/864bfc91 Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/864bfc91 Branch: refs/heads/master Commit: 864bfc9123f43856fdcd15cae5aa9c4435f5bf1f Parents: c7604ea Author: Matthias Boehm <[email protected]> Authored: Sun Jan 28 13:59:06 2018 -0800 Committer: Matthias Boehm <[email protected]> Committed: Sun Jan 28 13:59:06 2018 -0800 ---------------------------------------------------------------------- .../sysml/parser/ParForStatementBlock.java | 12 ++++++++--- .../parfor/opt/OptimizerRuleBased.java | 22 +++++++++----------- 2 files changed, 19 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/864bfc91/src/main/java/org/apache/sysml/parser/ParForStatementBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/ParForStatementBlock.java b/src/main/java/org/apache/sysml/parser/ParForStatementBlock.java index 6a39c7a..1c1621a 100644 --- a/src/main/java/org/apache/sysml/parser/ParForStatementBlock.java +++ b/src/main/java/org/apache/sysml/parser/ParForStatementBlock.java @@ -21,6 +21,7 @@ package org.apache.sysml.parser; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -1812,9 +1813,9 @@ public class ParForStatementBlock extends ForStatementBlock } @Override public boolean equals(Object that) { - if( !(that instanceof ResultVar) ) - return false; - return _name.equals(((ResultVar)that)._name); + String varname = (that instanceof ResultVar) ? + ((ResultVar)that)._name : that.toString(); + return _name.equals(varname); } @Override public int hashCode() { @@ -1824,6 +1825,11 @@ public class ParForStatementBlock extends ForStatementBlock public String toString() { return _name; } + public static boolean contains(Collection<ResultVar> list, String varName) { + //helper function which is necessary because list.contains checks + //varName.equals(rvar) which always returns false because it not a string + return list.stream().anyMatch(rvar -> rvar.equals(varName)); + } } private static class Candidate { http://git-wip-us.apache.org/repos/asf/systemml/blob/864bfc91/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/OptimizerRuleBased.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/OptimizerRuleBased.java b/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/OptimizerRuleBased.java index 9fc0dbe..4ce7466 100644 --- a/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/OptimizerRuleBased.java +++ b/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/OptimizerRuleBased.java @@ -657,7 +657,7 @@ public class OptimizerRuleBased extends Optimizer base = h.getInput().get(0); //check result variable - if( !resultVars.contains(base.getName()) ) + if( !ResultVar.contains(resultVars, base.getName()) ) ret = false; } @@ -1734,7 +1734,7 @@ public class OptimizerRuleBased extends Optimizer } else if( n.getNodeType()== NodeType.HOP) { Hop h = OptTreeConverter.getAbstractPlanMapping().getMappedHop(n.getID()); - if( h instanceof LeftIndexingOp && retVars.contains( h.getInput().get(0).getName() ) ) + if( h instanceof LeftIndexingOp && ResultVar.contains(retVars, h.getInput().get(0).getName() ) ) ret &= (h.getParent().size()==1 && h.getParent().get(0).getName().equals(h.getInput().get(0).getName())); } @@ -1811,7 +1811,7 @@ public class OptimizerRuleBased extends Optimizer if( h.getInput() != null ) for( Hop cn : h.getInput() ) if( cn instanceof DataOp && ((DataOp)cn).isRead() //read data - && !inplaceResultVars.contains(cn.getName())) //except in-place result vars + && !ResultVar.contains(inplaceResultVars, cn.getName())) //except in-place result vars sum += cn.getMemEstimate(); } } @@ -1868,15 +1868,13 @@ public class OptimizerRuleBased extends Optimizer if( ch instanceof DataOp && ch.getDataType() == DataType.MATRIX && inputVars.contains(ch.getName()) ) - //&& !partitionedVars.contains(ch.getName())) { ret = true; sharedVars.add(ch.getName()); } - else if( HopRewriteUtils.isTransposeOperation(ch) + else if( HopRewriteUtils.isTransposeOperation(ch) && ch.getInput().get(0) instanceof DataOp && ch.getInput().get(0).getDataType() == DataType.MATRIX && inputVars.contains(ch.getInput().get(0).getName()) ) - //&& !partitionedVars.contains(ch.getInput().get(0).getName())) { ret = true; sharedVars.add(ch.getInput().get(0).getName()); @@ -2182,7 +2180,7 @@ public class OptimizerRuleBased extends Optimizer throws DMLRuntimeException { ParForProgramBlock pfpb = (ParForProgramBlock) OptTreeConverter - .getAbstractPlanMapping().getMappedProg(n.getID())[1]; + .getAbstractPlanMapping().getMappedProg(n.getID())[1]; PResultMerge REMOTE = OptimizerUtils.isSparkExecutionMode() ? PResultMerge.REMOTE_SPARK : PResultMerge.REMOTE_MR; @@ -2224,7 +2222,7 @@ public class OptimizerRuleBased extends Optimizer pfpb.setResultMerge(ret); // modify plan - n.addParam(ParamType.RESULT_MERGE, ret.toString()); + n.addParam(ParamType.RESULT_MERGE, ret.toString()); //recursively apply rewrite for parfor nodes if( n.getChilds() != null ) @@ -2278,7 +2276,7 @@ public class OptimizerRuleBased extends Optimizer LeftIndexingOp hop = (LeftIndexingOp) OptTreeConverter.getAbstractPlanMapping().getMappedHop(n.getID()); //check agains set of varname String varName = hop.getInput().get(0).getName(); - if( resultVars.contains(varName) ) + if( ResultVar.contains(resultVars, varName) ) { ret = true; if( checkSize && vars.keySet().contains(varName) ) @@ -2324,7 +2322,7 @@ public class OptimizerRuleBased extends Optimizer for( ResultVar var : resultVars ) { //Potential unknowns: for local result var of child parfor (but we're only interested in top level) - //Potential scalars: for disabled dependency analysis and unbounded scoping + //Potential scalars: for disabled dependency analysis and unbounded scoping Data dat = vars.get( var._name ); if( dat != null && dat instanceof MatrixObject ) { @@ -2380,7 +2378,7 @@ public class OptimizerRuleBased extends Optimizer LeftIndexingOp hop = (LeftIndexingOp) OptTreeConverter.getAbstractPlanMapping().getMappedHop(n.getID()); //check agains set of varname String varName = hop.getInput().get(0).getName(); - if( resultVars.contains(varName) && vars.keySet().contains(varName) ) + if( ResultVar.contains(resultVars, varName) && vars.keySet().contains(varName) ) { //dims of result vars must be known at this point in time MatrixObject mo = (MatrixObject) vars.get( hop.getInput().get(0).getName() ); @@ -2472,7 +2470,7 @@ public class OptimizerRuleBased extends Optimizer try { ParForProgramBlock pfpb = (ParForProgramBlock) OptTreeConverter - .getAbstractPlanMapping().getMappedProg(n.getID())[1]; + .getAbstractPlanMapping().getMappedProg(n.getID())[1]; if( recPBs.contains(pfpb) ) rFindAndUnfoldRecursiveFunction(n, pfpb, recPBs, vars); }
