[MINOR] Performance nrow/ncol runtime instructions (parsing overhead) This patch makes a minor performance improvement to the very common nrow and ncol instructions by (1) avoiding repeated opcode string comparisons on instruction execution, and (2) cleaning up the unnecessarily convoluted code path. On a multi-threaded scenario of 240,000,000 nrow operations, this patch improved the total runtime from 88.5 to 65.2s
Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/0a0a4037 Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/0a0a4037 Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/0a0a4037 Branch: refs/heads/master Commit: 0a0a403703237be548510f98c2c871da72452e0c Parents: 151f658 Author: Matthias Boehm <[email protected]> Authored: Mon Jan 15 20:58:06 2018 -0800 Committer: Matthias Boehm <[email protected]> Committed: Mon Jan 15 20:58:06 2018 -0800 ---------------------------------------------------------------------- .../cp/AggregateUnaryCPInstruction.java | 94 +++++++++----------- .../cp/CentralMomentCPInstruction.java | 2 +- 2 files changed, 45 insertions(+), 51 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/0a0a4037/src/main/java/org/apache/sysml/runtime/instructions/cp/AggregateUnaryCPInstruction.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/instructions/cp/AggregateUnaryCPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/cp/AggregateUnaryCPInstruction.java index d57a8dd..c6e3cce 100644 --- a/src/main/java/org/apache/sysml/runtime/instructions/cp/AggregateUnaryCPInstruction.java +++ b/src/main/java/org/apache/sysml/runtime/instructions/cp/AggregateUnaryCPInstruction.java @@ -35,34 +35,44 @@ import org.apache.sysml.runtime.matrix.operators.AggregateUnaryOperator; import org.apache.sysml.runtime.matrix.operators.Operator; import org.apache.sysml.runtime.matrix.operators.SimpleOperator; -public class AggregateUnaryCPInstruction extends UnaryCPInstruction { - - private AggregateUnaryCPInstruction(Operator op, CPOperand in, CPOperand out, String opcode, String istr) { - this(op, in, null, null, out, opcode, istr); +public class AggregateUnaryCPInstruction extends UnaryCPInstruction +{ + public enum AUType { + NROW, NCOL, LENGTH, + DEFAULT; + public boolean isMeta() { + return this != DEFAULT; + } + } + + private final AUType _type; + + private AggregateUnaryCPInstruction(Operator op, CPOperand in, CPOperand out, AUType type, String opcode, String istr) { + this(op, in, null, null, out, type, opcode, istr); } protected AggregateUnaryCPInstruction(Operator op, CPOperand in1, CPOperand in2, CPOperand in3, CPOperand out, - String opcode, String istr) { + AUType type, String opcode, String istr) { super(CPType.AggregateUnary, op, in1, in2, in3, out, opcode, istr); + _type = type; } - + public static AggregateUnaryCPInstruction parseInstruction(String str) throws DMLRuntimeException { String[] parts = InstructionUtils.getInstructionPartsWithValueType(str); - String opcode = parts[0]; + String opcode = parts[0]; CPOperand in1 = new CPOperand(parts[1]); CPOperand out = new CPOperand(parts[2]); if(opcode.equalsIgnoreCase("nrow") || opcode.equalsIgnoreCase("ncol") || opcode.equalsIgnoreCase("length")){ return new AggregateUnaryCPInstruction(new SimpleOperator(Builtin.getBuiltinFnObject(opcode)), - in1, out, opcode, str); + in1, out, AUType.valueOf(opcode.toUpperCase()), opcode, str); } - else //DEFAULT BEHAVIOR - { + else { //DEFAULT BEHAVIOR AggregateUnaryOperator aggun = InstructionUtils .parseBasicAggregateUnaryOperator(opcode, Integer.parseInt(parts[3])); - return new AggregateUnaryCPInstruction(aggun, in1, out, opcode, str); + return new AggregateUnaryCPInstruction(aggun, in1, out, AUType.DEFAULT, opcode, str); } } @@ -73,22 +83,15 @@ public class AggregateUnaryCPInstruction extends UnaryCPInstruction { String output_name = output.getName(); String opcode = getOpcode(); - if( opcode.equalsIgnoreCase("nrow") || opcode.equalsIgnoreCase("ncol") || opcode.equalsIgnoreCase("length") ) + if( _type.isMeta() ) //nrow/ncol/length { //check existence of input variable - if( !ec.getVariables().keySet().contains(input1.getName()) ){ + if( !ec.getVariables().keySet().contains(input1.getName()) ) throw new DMLRuntimeException("Variable '"+input1.getName()+"' does not exist."); - } //get meta data information MatrixCharacteristics mc = ec.getMatrixCharacteristics(input1.getName()); - long rval = -1; - if(opcode.equalsIgnoreCase("nrow")) - rval = mc.getRows(); - else if(opcode.equalsIgnoreCase("ncol")) - rval = mc.getCols(); - else if(opcode.equalsIgnoreCase("length")) - rval = mc.getRows() * mc.getCols(); + long rval = getSizeMetaData(_type, mc); //check for valid output, and acquire read if necessary //(Use case: In case of forced exec type singlenode, there are no reblocks. For csv @@ -97,12 +100,11 @@ public class AggregateUnaryCPInstruction extends UnaryCPInstruction { //Note: check on matrix characteristics to cover incorrect length (-1*-1 -> 1) if( !mc.dimsKnown() ) //invalid nrow/ncol/length { - if( DMLScript.rtplatform == RUNTIME_PLATFORM.SINGLE_NODE + if( DMLScript.rtplatform == RUNTIME_PLATFORM.SINGLE_NODE || (input1.getDataType() == DataType.FRAME && OptimizerUtils.isHadoopExecutionMode()) ) { - if( OptimizerUtils.isHadoopExecutionMode() ) { + if( OptimizerUtils.isHadoopExecutionMode() ) LOG.warn("Reading csv input frame of unkown size into memory for '"+opcode+"'."); - } //read the input matrix/frame and explicitly refresh meta data CacheableData<?> obj = ec.getCacheableData(input1.getName()); @@ -112,12 +114,7 @@ public class AggregateUnaryCPInstruction extends UnaryCPInstruction { //update meta data information mc = ec.getMatrixCharacteristics(input1.getName()); - if(opcode.equalsIgnoreCase("nrow")) - rval = mc.getRows(); - else if(opcode.equalsIgnoreCase("ncol")) - rval = mc.getCols(); - else if(opcode.equalsIgnoreCase("length")) - rval = mc.getRows() * mc.getCols(); + rval = getSizeMetaData(_type, mc); } else { throw new DMLRuntimeException("Invalid meta data returned by '"+opcode+"': "+rval + ":" + instString); @@ -125,36 +122,33 @@ public class AggregateUnaryCPInstruction extends UnaryCPInstruction { } //create and set output scalar - ScalarObject ret = null; - switch( output.getValueType() ) { - case INT: ret = new IntObject(rval); break; - case DOUBLE: ret = new DoubleObject(rval); break; - case STRING: ret = new StringObject(String.valueOf(rval)); break; - - default: - throw new DMLRuntimeException("Invalid output value type: "+output.getValueType()); - } - ec.setScalarOutput(output_name, ret); - return; + ec.setScalarOutput(output_name, new IntObject(rval)); } - else - { - /* Default behavior for AggregateUnary Instruction */ - MatrixBlock matBlock = ec.getMatrixInput(input1.getName(), getExtendedOpcode()); + else { //DEFAULT + MatrixBlock matBlock = ec.getMatrixInput(input1.getName()); AggregateUnaryOperator au_op = (AggregateUnaryOperator) _optr; - MatrixBlock resultBlock = (MatrixBlock) matBlock.aggregateUnaryOperations(au_op, new MatrixBlock(), matBlock.getNumRows(), matBlock.getNumColumns(), new MatrixIndexes(1, 1), true); - - ec.releaseMatrixInput(input1.getName(), getExtendedOpcode()); + MatrixBlock resultBlock = (MatrixBlock) matBlock.aggregateUnaryOperations(au_op, new MatrixBlock(), + matBlock.getNumRows(), matBlock.getNumColumns(), new MatrixIndexes(1, 1), true); + ec.releaseMatrixInput(input1.getName()); if(output.getDataType() == DataType.SCALAR){ DoubleObject ret = new DoubleObject(resultBlock.getValue(0, 0)); ec.setScalarOutput(output_name, ret); } else{ // since the computed value is a scalar, allocate a "temp" output matrix - ec.setMatrixOutput(output_name, resultBlock, getExtendedOpcode()); + ec.setMatrixOutput(output_name, resultBlock); } } } - + + private static long getSizeMetaData(AUType type, MatrixCharacteristics mc) { + switch( type ) { + case NROW: return mc.getRows(); + case NCOL: return mc.getCols(); + case LENGTH: return mc.getRows() * mc.getCols(); + default: + throw new RuntimeException("Opcode not applicable: "+type.name()); + } + } } http://git-wip-us.apache.org/repos/asf/systemml/blob/0a0a4037/src/main/java/org/apache/sysml/runtime/instructions/cp/CentralMomentCPInstruction.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/instructions/cp/CentralMomentCPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/cp/CentralMomentCPInstruction.java index c13ca4c..1bd5b60 100644 --- a/src/main/java/org/apache/sysml/runtime/instructions/cp/CentralMomentCPInstruction.java +++ b/src/main/java/org/apache/sysml/runtime/instructions/cp/CentralMomentCPInstruction.java @@ -33,7 +33,7 @@ public class CentralMomentCPInstruction extends AggregateUnaryCPInstruction { private CentralMomentCPInstruction(CMOperator cm, CPOperand in1, CPOperand in2, CPOperand in3, CPOperand out, String opcode, String str) { - super(cm, in1, in2, in3, out, opcode, str); + super(cm, in1, in2, in3, out, AUType.DEFAULT, opcode, str); } public static CentralMomentCPInstruction parseInstruction(String str)
