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 2018/02/20 04:06:47 UTC

[1/3] systemml git commit: [SYSTEMML-2154] Fix misleading explain recompile hops output

Repository: systemml
Updated Branches:
  refs/heads/master e9a6e396a -> 2c9418c3e


[SYSTEMML-2154] Fix misleading explain recompile hops output

Since a consolidation of duplicated recompilation code paths in
SYSTEMML-2072, explain recompile_hops returns misleading outputs.
Specifically, the recompiler creates a deep copy of the hop dag, but the
explain output shows the original hop dag, which leads to understandable
confusion. This patch fixes this issue, while preserving the
consolidated recompilation code path.


Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/bd9d7eb0
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/bd9d7eb0
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/bd9d7eb0

Branch: refs/heads/master
Commit: bd9d7eb00ebd60656042a312e3cdba30470d36cb
Parents: e9a6e39
Author: Matthias Boehm <mb...@gmail.com>
Authored: Mon Feb 19 14:39:08 2018 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Mon Feb 19 20:06:40 2018 -0800

----------------------------------------------------------------------
 .../apache/sysml/hops/recompile/Recompiler.java | 49 ++++++++++++++------
 1 file changed, 34 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/bd9d7eb0/src/main/java/org/apache/sysml/hops/recompile/Recompiler.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/recompile/Recompiler.java b/src/main/java/org/apache/sysml/hops/recompile/Recompiler.java
index 13bd81c..ca9546f 100644
--- a/src/main/java/org/apache/sysml/hops/recompile/Recompiler.java
+++ b/src/main/java/org/apache/sysml/hops/recompile/Recompiler.java
@@ -160,7 +160,7 @@ public class Recompiler
 		//need for synchronization as we do temp changes in shared hops/lops
 		//however, we create deep copies for most dags to allow for concurrent recompile
 		synchronized( hops ) {
-			newInst = recompile(sb, hops, vars, status, inplace, replaceLit, true, false, null, tid);
+			newInst = recompile(sb, hops, vars, status, inplace, replaceLit, true, false, false, null, tid);
 		}
 		
 		// replace thread ids in new instructions
@@ -172,7 +172,8 @@ public class Recompiler
 			newInst = JMLCUtils.cleanupRuntimeInstructions(newInst, vars.getRegisteredOutputs());
 		
 		// explain recompiled hops / instructions
-		logExplainDAG(sb, hops, newInst);
+		if( DMLScript.EXPLAIN == ExplainType.RECOMPILE_RUNTIME )
+			logExplainDAG(sb, hops, newInst);
 	
 		return newInst;
 	}
@@ -186,7 +187,7 @@ public class Recompiler
 		//need for synchronization as we do temp changes in shared hops/lops
 		synchronized( hop ) {
 			newInst = recompile(null, new ArrayList<>(Arrays.asList(hop)),
-				vars, status, inplace, replaceLit, true, false, null, tid);
+				vars, status, inplace, replaceLit, true, false, true, null, tid);
 		}
 		
 		// replace thread ids in new instructions
@@ -194,7 +195,8 @@ public class Recompiler
 			newInst = ProgramConverter.createDeepCopyInstructionSet(newInst, tid, -1, null, null, null, false, false);
 		
 		// explain recompiled instructions
-		logExplainPred(hop, newInst);
+		if( DMLScript.EXPLAIN == ExplainType.RECOMPILE_RUNTIME )
+			logExplainPred(hop, newInst);
 		
 		return newInst;
 	}
@@ -208,7 +210,7 @@ public class Recompiler
 		//however, we create deep copies for most dags to allow for concurrent recompile
 		synchronized( hops ) {
 			//always in place, no stats update/rewrites, but forced exec type
-			newInst = recompile(sb, hops, null, null, true, false, false, true, et, tid);
+			newInst = recompile(sb, hops, null, null, true, false, false, true, false, et, tid);
 		}
 		
 		// replace thread ids in new instructions
@@ -216,7 +218,8 @@ public class Recompiler
 			newInst = ProgramConverter.createDeepCopyInstructionSet(newInst, tid, -1, null, null, null, false, false);
 		
 		// explain recompiled hops / instructions
-		logExplainDAG(sb, hops, newInst);
+		if( DMLScript.EXPLAIN == ExplainType.RECOMPILE_RUNTIME )
+			logExplainDAG(sb, hops, newInst);
 		
 		return newInst;
 	}
@@ -230,7 +233,7 @@ public class Recompiler
 		synchronized( hop ) {
 			//always in place, no stats update/rewrites, but forced exec type
 			newInst = recompile(null, new ArrayList<>(Arrays.asList(hop)),
-				null, null, true, false, false, true, et, tid);
+				null, null, true, false, false, true, true, et, tid);
 		}
 
 		// replace thread ids in new instructions
@@ -238,7 +241,8 @@ public class Recompiler
 			newInst = ProgramConverter.createDeepCopyInstructionSet(newInst, tid, -1, null, null, null, false, false);
 		
 		// explain recompiled hops / instructions
-		logExplainPred(hop, newInst);
+		if( DMLScript.EXPLAIN == ExplainType.RECOMPILE_RUNTIME )
+			logExplainPred(hop, newInst);
 		
 		return newInst;
 	}
@@ -252,11 +256,12 @@ public class Recompiler
 		//however, we create deep copies for most dags to allow for concurrent recompile
 		synchronized( hops ) {
 			//always in place, no stats update/rewrites
-			newInst = recompile(sb, hops, null, null, true, false, false, false, null, 0);
+			newInst = recompile(sb, hops, null, null, true, false, false, false, false, null, 0);
 		}
 		
 		// explain recompiled hops / instructions
-		logExplainDAG(sb, hops, newInst);
+		if( DMLScript.EXPLAIN == ExplainType.RECOMPILE_RUNTIME )
+			logExplainDAG(sb, hops, newInst);
 		
 		return newInst;
 	}
@@ -270,11 +275,12 @@ public class Recompiler
 		synchronized( hop ) {
 			//always in place, no stats update/rewrites
 			newInst = recompile(null, new ArrayList<>(Arrays.asList(hop)),
-				null, null, true, false, false, false, null, 0);
+				null, null, true, false, false, false, true, null, 0);
 		}
 		
 		// explain recompiled instructions
-		logExplainPred(hop, newInst);
+		if( DMLScript.EXPLAIN == ExplainType.RECOMPILE_RUNTIME )
+			logExplainPred(hop, newInst);
 		
 		return newInst;
 	}
@@ -291,6 +297,7 @@ public class Recompiler
 	 * @param replaceLit replace literals (only applicable on deep copy)
 	 * @param updateStats update statistics, rewrites, and memory estimates
 	 * @param forceEt force a given execution type, null for reset
+	 * @param pred recompile for predicate DAG
 	 * @param et given execution type
 	 * @param tid thread id, 0 for main or before worker creation
 	 * @return modified list of instructions
@@ -298,8 +305,8 @@ public class Recompiler
 	 * @throws LopsException if lop compile error
 	 * @throws DMLRuntimeException if runtime error on literal replacement
 	 */
-	private static ArrayList<Instruction> recompile(StatementBlock sb, ArrayList<Hop> hops, LocalVariableMap vars,
-		RecompileStatus status, boolean inplace, boolean replaceLit, boolean updateStats, boolean forceEt, ExecType et, long tid ) 
+	private static ArrayList<Instruction> recompile(StatementBlock sb, ArrayList<Hop> hops, LocalVariableMap vars, RecompileStatus status,
+		boolean inplace, boolean replaceLit, boolean updateStats, boolean forceEt, boolean pred, ExecType et, long tid ) 
 			throws HopsException, LopsException, DMLRuntimeException
 	{
 		// prepare hops dag for recompile
@@ -378,7 +385,19 @@ public class Recompiler
 		}
 		
 		// generate runtime instructions (incl piggybacking)
-		return dag.getJobs(sb, ConfigurationManager.getDMLConfig());
+		ArrayList<Instruction> newInst = dag
+			.getJobs(sb, ConfigurationManager.getDMLConfig());
+		
+		// explain recompiled (and potentially deep copied) DAG, but
+		// defer the explain of instructions after additional modifications
+		if( DMLScript.EXPLAIN == ExplainType.RECOMPILE_HOPS ) {
+			if( pred )
+				logExplainPred(hops.get(0), newInst);
+			else
+				logExplainDAG(sb, hops, newInst);
+		}
+		
+		return newInst;
 	}
 	
 	private static void logExplainDAG(StatementBlock sb, ArrayList<Hop> hops, ArrayList<Instruction> inst)


[2/3] systemml git commit: [SYSTEMML-2155] Recompute nnz on spark checkpoints for large matrices

Posted by mb...@apache.org.
[SYSTEMML-2155] Recompute nnz on spark checkpoints for large matrices

This patch extends the existing spark checkpoint instruction (caching)
by the recomputation of nnz for matrices with dimensions larger than max
integer. Such matrices are always accessed by spark instructions and
hence never get their nnz computed due to lazy evaluation in spark.
Doing this nnz computation on checkpoints incurs almost no risk of
additional overhead because the intermediate will anyway be checkpointed
and is likely accessed many times.


Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/b0fff8c1
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/b0fff8c1
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/b0fff8c1

Branch: refs/heads/master
Commit: b0fff8c18e9aaa7be188f0defdefe251a3a4d46d
Parents: bd9d7eb
Author: Matthias Boehm <mb...@gmail.com>
Authored: Mon Feb 19 15:51:17 2018 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Mon Feb 19 20:06:43 2018 -0800

----------------------------------------------------------------------
 .../java/org/apache/sysml/hops/OptimizerUtils.java     |  5 +++++
 .../instructions/spark/CheckpointSPInstruction.java    | 13 ++++++++++---
 .../runtime/instructions/spark/utils/SparkUtils.java   |  4 ++++
 3 files changed, 19 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/b0fff8c1/src/main/java/org/apache/sysml/hops/OptimizerUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/hops/OptimizerUtils.java b/src/main/java/org/apache/sysml/hops/OptimizerUtils.java
index 224752d..3a406fc 100644
--- a/src/main/java/org/apache/sysml/hops/OptimizerUtils.java
+++ b/src/main/java/org/apache/sysml/hops/OptimizerUtils.java
@@ -826,6 +826,11 @@ public class OptimizerUtils
 				|| (rl-1)/brlen == (ru-1)/brlen && (cl-1)%bclen == 0 
 				|| (rl-1)%brlen == 0 && (cl-1)/bclen == (cu-1)/bclen);
 	}
+	
+	public static boolean isValidCPDimensions( MatrixCharacteristics mc ) {
+		return isValidCPDimensions(mc.getRows(), mc.getCols());
+	}
+	
 	/**
 	 * Returns false if dimensions known to be invalid; other true
 	 * 

http://git-wip-us.apache.org/repos/asf/systemml/blob/b0fff8c1/src/main/java/org/apache/sysml/runtime/instructions/spark/CheckpointSPInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/spark/CheckpointSPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/spark/CheckpointSPInstruction.java
index 3b71948..90f6c7b 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/spark/CheckpointSPInstruction.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/spark/CheckpointSPInstruction.java
@@ -130,19 +130,26 @@ public class CheckpointSPInstruction extends UnarySPInstruction {
 			//convert mcsr into memory-efficient csr if potentially sparse
 			if( input1.getDataType()==DataType.MATRIX 
 				&& OptimizerUtils.checkSparseBlockCSRConversion(mcIn)
-				&& !_level.equals(Checkpoint.SER_STORAGE_LEVEL) ) 
-			{				
+				&& !_level.equals(Checkpoint.SER_STORAGE_LEVEL) ) {
 				out = ((JavaPairRDD<MatrixIndexes,MatrixBlock>)out)
 					.mapValues(new CreateSparseBlockFunction(SparseBlock.Type.CSR));
 			}
 			
 			//actual checkpoint into given storage level
 			out = out.persist( _level );
+			
+			//trigger nnz computation for datasets that are forced to spark by their dimensions
+			//(larger than MAX_INT) to handle ultra-sparse data sets during recompilation because
+			//otherwise these their nnz would never be evaluated due to lazy evaluation in spark
+			if( input1.isMatrix() && mcIn.dimsKnown() 
+				&& !mcIn.dimsKnown(true) && !OptimizerUtils.isValidCPDimensions(mcIn) ) {
+				mcIn.setNonZeros(SparkUtils.getNonZeros((JavaPairRDD<MatrixIndexes,MatrixBlock>)out));
+			}
 		}
 		else {
 			out = in; //pass-through
 		}
-			
+		
 		// Step 3: In-place update of input matrix/frame rdd handle and set as output
 		// -------
 		// We use this in-place approach for two reasons. First, it is correct because our checkpoint 

http://git-wip-us.apache.org/repos/asf/systemml/blob/b0fff8c1/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/SparkUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/SparkUtils.java b/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/SparkUtils.java
index 801fe5a..b24d56f 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/SparkUtils.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/spark/utils/SparkUtils.java
@@ -232,6 +232,10 @@ public class SparkUtils
 		
 		return ret;
 	}
+	
+	public static long getNonZeros(JavaPairRDD<MatrixIndexes, MatrixBlock> input) {
+		return input.values().map(b -> b.getNonZeros()).reduce((a,b)->a+b);
+	}
 
 	private static class AnalyzeCellMatrixCharacteristics implements Function<Tuple2<MatrixIndexes,MatrixCell>, MatrixCharacteristics> 
 	{


[3/3] systemml git commit: [SYSTEMML-2157] Fix parfor optimizer side effect on codegen plans

Posted by mb...@apache.org.
[SYSTEMML-2157] Fix parfor optimizer side effect on codegen plans

The parfor optimizer applies a variety of rewrites some which require
the recompilation of the parfor body program or parts of it. Most of
these recompilations leverage the recompiler and thus work properly with
codegen. However, there are two rewrites (removal of branches and the
injection of spark checkpoints) that need to recreate not just
instructions but a partial runtime program. This code path was not
integrated with codegen yet and hence led to lost fusion plans whenever
these rewrites triggered (e.g., for Kmeans over a specific range of data
sizes). This patch fixes the underlying code path for partial program
recompilations, which ensures robustness independent of concrete
instances of rewrites. We also modified the Kmeans tests to check for
properly compiled row-wise templates in the inner loop.


Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/2c9418c3
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/2c9418c3
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/2c9418c3

Branch: refs/heads/master
Commit: 2c9418c3ec6d81dbb0b11c05e56d2672f1992edd
Parents: b0fff8c
Author: Matthias Boehm <mb...@gmail.com>
Authored: Mon Feb 19 20:06:10 2018 -0800
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Mon Feb 19 20:06:46 2018 -0800

----------------------------------------------------------------------
 src/main/java/org/apache/sysml/parser/DMLTranslator.java    | 6 ++++++
 .../controlprogram/parfor/opt/ProgramRecompiler.java        | 9 +++++++++
 .../integration/functions/codegenalg/AlgorithmKMeans.java   | 3 ++-
 3 files changed, 17 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/systemml/blob/2c9418c3/src/main/java/org/apache/sysml/parser/DMLTranslator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/parser/DMLTranslator.java b/src/main/java/org/apache/sysml/parser/DMLTranslator.java
index 8437974..63c896c 100644
--- a/src/main/java/org/apache/sysml/parser/DMLTranslator.java
+++ b/src/main/java/org/apache/sysml/parser/DMLTranslator.java
@@ -308,6 +308,12 @@ public class DMLTranslator
 		SpoofCompiler.generateCode(rtprog);
 	}
 	
+	public void codgenHopsDAG(ProgramBlock pb)
+		throws HopsException, DMLRuntimeException, LopsException, IOException 
+	{
+		SpoofCompiler.generateCodeFromProgramBlock(pb);
+	}
+	
 	public void constructLops(DMLProgram dmlp) throws ParseException, LanguageException, HopsException, LopsException {
 		// for each namespace, handle function program blocks handle function 
 		for( String namespaceKey : dmlp.getNamespaces().keySet() )

http://git-wip-us.apache.org/repos/asf/systemml/blob/2c9418c3/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/ProgramRecompiler.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/ProgramRecompiler.java b/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/ProgramRecompiler.java
index d16d708..3bb0c93 100644
--- a/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/ProgramRecompiler.java
+++ b/src/main/java/org/apache/sysml/runtime/controlprogram/parfor/opt/ProgramRecompiler.java
@@ -28,6 +28,8 @@ import org.apache.sysml.hops.Hop;
 import org.apache.sysml.hops.HopsException;
 import org.apache.sysml.hops.IndexingOp;
 import org.apache.sysml.hops.OptimizerUtils;
+import org.apache.sysml.hops.codegen.SpoofCompiler;
+import org.apache.sysml.hops.codegen.SpoofCompiler.IntegrationType;
 import org.apache.sysml.hops.recompile.Recompiler;
 import org.apache.sysml.lops.Lop;
 import org.apache.sysml.lops.LopProperties;
@@ -74,6 +76,13 @@ public class ProgramRecompiler
 			ret.add(dmlt.createRuntimeProgramBlock(rtprog, sb, config));
 		}
 		
+		//enhance runtime program by automatic operator fusion
+		if( ConfigurationManager.isCodegenEnabled() 
+			&& SpoofCompiler.INTEGRATION==IntegrationType.RUNTIME ) {
+			for( ProgramBlock pb : ret )
+				dmlt.codgenHopsDAG(pb);
+		}
+		
 		return ret;
 	}
 	

http://git-wip-us.apache.org/repos/asf/systemml/blob/2c9418c3/src/test/java/org/apache/sysml/test/integration/functions/codegenalg/AlgorithmKMeans.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/codegenalg/AlgorithmKMeans.java b/src/test/java/org/apache/sysml/test/integration/functions/codegenalg/AlgorithmKMeans.java
index c131ea8..f04d684 100644
--- a/src/test/java/org/apache/sysml/test/integration/functions/codegenalg/AlgorithmKMeans.java
+++ b/src/test/java/org/apache/sysml/test/integration/functions/codegenalg/AlgorithmKMeans.java
@@ -172,7 +172,8 @@ public class AlgorithmKMeans extends AutomatedTestBase
 			
 			runTest(true, false, null, -1); 
 			
-			Assert.assertTrue(heavyHittersContainsSubString("spoof") || heavyHittersContainsSubString("sp_spoof"));
+			Assert.assertTrue(heavyHittersContainsSubString("spoofCell") || heavyHittersContainsSubString("sp_spoofCell"));
+			Assert.assertTrue(heavyHittersContainsSubString("spoofRA") || heavyHittersContainsSubString("sp_spoofRA"));
 		}
 		finally {
 			rtplatform = platformOld;