Repository: systemml Updated Branches: refs/heads/master de69afdc8 -> a9c14b02b
[SYSTEMML-2014] New optional sideeffect parameter for external UDFs This patch introduces a new optional sideeffect parameter for external UDFs that write or read data. Making sideeffects explicit allows more aggressive rewrites wrt merging statement blocks that create more opportunities for common subexpression elimination and rewrites. Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/4fea3c6d Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/4fea3c6d Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/4fea3c6d Branch: refs/heads/master Commit: 4fea3c6d42ccecadb826be597a4066b33ec77bd6 Parents: de69afd Author: Matthias Boehm <[email protected]> Authored: Mon Nov 13 19:01:29 2017 -0800 Committer: Matthias Boehm <[email protected]> Committed: Tue Nov 14 11:54:25 2017 -0800 ---------------------------------------------------------------------- .../hops/rewrite/RewriteMergeBlockSequence.java | 8 ++-- .../sysml/parser/ExternalFunctionStatement.java | 41 ++++++++++++-------- .../scripts/functions/external/DynReadWrite.dml | 4 +- 3 files changed, 32 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/4fea3c6d/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java b/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java index 8742ade..775006d 100644 --- a/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java +++ b/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java @@ -68,7 +68,8 @@ public class RewriteMergeBlockSequence extends StatementBlockRewriteRule if( HopRewriteUtils.isLastLevelStatementBlock(sb1) && HopRewriteUtils.isLastLevelStatementBlock(sb2) && !sb1.isSplitDag() && !sb2.isSplitDag() - && !(hasExternalFunctionOpRoot(sb1) && hasExternalFunctionOpRoot(sb2)) + && !(hasExternalFunctionOpRootWithSideEffect(sb1) + && hasExternalFunctionOpRootWithSideEffect(sb2)) && (!hasFunctionOpRoot(sb1) || !hasFunctionIOConflict(sb1,sb2)) && (!hasFunctionOpRoot(sb2) || !hasFunctionIOConflict(sb2,sb1)) ) { @@ -170,7 +171,7 @@ public class RewriteMergeBlockSequence extends StatementBlockRewriteRule return ret; } - private static boolean hasExternalFunctionOpRoot(StatementBlock sb) + private static boolean hasExternalFunctionOpRootWithSideEffect(StatementBlock sb) throws HopsException { if( sb == null || sb.getHops() == null ) return false; @@ -180,7 +181,8 @@ public class RewriteMergeBlockSequence extends StatementBlockRewriteRule .getFunctionStatementBlock(((FunctionOp)root).getFunctionKey()); //note: in case of builtin multi-return functions such as qr (namespace _internal), //there is no function statement block and hence we need to check for null - if( fsb != null && fsb.getStatement(0) instanceof ExternalFunctionStatement ) + if( fsb != null && fsb.getStatement(0) instanceof ExternalFunctionStatement + && ((ExternalFunctionStatement)fsb.getStatement(0)).hasSideEffects() ) return true; } return false; http://git-wip-us.apache.org/repos/asf/systemml/blob/4fea3c6d/src/main/java/org/apache/sysml/parser/ExternalFunctionStatement.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/parser/ExternalFunctionStatement.java b/src/main/java/org/apache/sysml/parser/ExternalFunctionStatement.java index 14cca05..c3e9a68 100644 --- a/src/main/java/org/apache/sysml/parser/ExternalFunctionStatement.java +++ b/src/main/java/org/apache/sysml/parser/ExternalFunctionStatement.java @@ -23,22 +23,20 @@ import java.util.HashMap; public class ExternalFunctionStatement extends FunctionStatement { - //valid attribute names public static final String CLASS_NAME = "classname"; public static final String EXEC_TYPE = "exectype"; - //public static final String EXEC_LOCATION = "execlocation"; //MB: obsolete public static final String CONFIG_FILE = "configfile"; + public static final String SIDE_EFFECTS = "sideeffect"; + - //valid attribute values for execlocation and + //valid attribute values for execlocation public static final String FILE_BASED = "file"; public static final String IN_MEMORY = "mem"; - //public static final String MASTER = "master"; //MB: obsolete - //public static final String WORKER = "worker"; //MB: obsolete //default values for optional attributes - public static final String DEFAULT_EXEC_TYPE = FILE_BASED; - //public static final String DEFAULT_EXEC_LOCATION = MASTER; //MB: obsolete + public static final String DEFAULT_EXEC_TYPE = FILE_BASED; + public static final String DEFAULT_SIDE_EFFECTS = "false"; //all parameters private HashMap<String,String> _otherParams; @@ -56,6 +54,11 @@ public class ExternalFunctionStatement extends FunctionStatement return _otherParams; } + public boolean hasSideEffects() { + return _otherParams.containsKey(SIDE_EFFECTS) + && Boolean.parseBoolean(_otherParams.get(SIDE_EFFECTS)); + } + /** * Validates all attributes and attribute values. * @@ -68,8 +71,8 @@ public class ExternalFunctionStatement extends FunctionStatement //warnings for all not defined attributes for( String varName : _otherParams.keySet() ) - if( !( varName.equals(CLASS_NAME) || varName.equals(EXEC_TYPE) - || varName.equals(CONFIG_FILE) ) ) + if( !(varName.equals(CLASS_NAME) || varName.equals(EXEC_TYPE) + || varName.equals(CONFIG_FILE) || varName.equals(SIDE_EFFECTS) ) ) { LOG.warn( printWarningLocation() + "External function specifies undefined attribute type '"+varName+"'."); } @@ -83,19 +86,25 @@ public class ExternalFunctionStatement extends FunctionStatement } //exec type (optional, default: file) - if( _otherParams.containsKey( EXEC_TYPE ) ) - { + if( _otherParams.containsKey( EXEC_TYPE ) ) { //check specified values String execType = _otherParams.get(EXEC_TYPE); - if( !(execType.equals(FILE_BASED) || execType.equals(IN_MEMORY)) ) { //always unconditional (invalid parameter) - sb.raiseValidateError("External function specifies invalid value for (optional) attribute '"+EXEC_TYPE+"' (valid values: "+FILE_BASED+","+IN_MEMORY+").", false); - } + if( !(execType.equals(FILE_BASED) || execType.equals(IN_MEMORY)) ) //always unconditional (invalid parameter) + sb.raiseValidateError("External function specifies invalid value for (optional) attribute '" + + EXEC_TYPE+"' (valid values: "+FILE_BASED+","+IN_MEMORY+").", false); } - else - { + else { //put default values _otherParams.put(EXEC_TYPE, DEFAULT_EXEC_TYPE); } + + //side effects + if( _otherParams.containsKey( SIDE_EFFECTS ) ) { + String sideeffect = _otherParams.get(SIDE_EFFECTS); + if( !(sideeffect.equals("true") || sideeffect.equals("false")) ) //always unconditional (invalid parameter) + sb.raiseValidateError("External function specifies invalid value for (optional) attribute '" + + SIDE_EFFECTS+"' (valid values: true, false).", false); + } } @Override http://git-wip-us.apache.org/repos/asf/systemml/blob/4fea3c6d/src/test/scripts/functions/external/DynReadWrite.dml ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/external/DynReadWrite.dml b/src/test/scripts/functions/external/DynReadWrite.dml index eb98991..e100277 100644 --- a/src/test/scripts/functions/external/DynReadWrite.dml +++ b/src/test/scripts/functions/external/DynReadWrite.dml @@ -22,11 +22,11 @@ dynRead = externalFunction(String fname, Integer rows, Integer cols, String format) return (Matrix[Double] M) -implemented in (classname="org.apache.sysml.udf.lib.DynamicReadMatrixCP",exectype="mem") +implemented in (classname="org.apache.sysml.udf.lib.DynamicReadMatrixCP",exectype="mem", sideeffect="true") dynWrite = externalFunction(Matrix[Double] input, String fname, String format) return(Boolean success) -implemented in (classname="org.apache.sysml.udf.lib.DynamicWriteMatrixCP",exectype="mem") +implemented in (classname="org.apache.sysml.udf.lib.DynamicWriteMatrixCP",exectype="mem", sideeffect="true") X = read($1, rows=$2, cols=$3, format="text");
