This is an automated email from the ASF dual-hosted git repository.
mboehm7 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/master by this push:
new 5fda23b [MINOR] Fix spark exec type selection for unary aggregates, II
5fda23b is described below
commit 5fda23bf94607da1c0ef609d303e32fc25fa26ad
Author: Matthias Boehm <[email protected]>
AuthorDate: Mon Jul 26 22:38:24 2021 +0200
[MINOR] Fix spark exec type selection for unary aggregates, II
This patch fixes an oversight of of the refined spark execution type
selection, which led to stackoverflow error in cyclic dependencies.
Imagine the case where op1 feeds into uagg1 and uagg2, both compiled
to CP. The two uagg2 would both probe if all remaining parents other
than themselfs are executed in spark which however is dependent on
the other uagg operator, leading to a cycle. We now call these checks
without transitive type inference and exented all hops accordingly.
Furthermore, the conditions have been reordered such that we only enter
these checks if the input is indeed already subject to spark and not
vice versa.
---
src/main/java/org/apache/sysds/hops/AggBinaryOp.java | 7 ++++---
src/main/java/org/apache/sysds/hops/AggUnaryOp.java | 10 +++++-----
src/main/java/org/apache/sysds/hops/BinaryOp.java | 4 ++--
src/main/java/org/apache/sysds/hops/DataGenOp.java | 2 +-
src/main/java/org/apache/sysds/hops/DataOp.java | 2 +-
src/main/java/org/apache/sysds/hops/DnnOp.java | 2 +-
src/main/java/org/apache/sysds/hops/FunctionOp.java | 2 +-
src/main/java/org/apache/sysds/hops/Hop.java | 6 +++++-
src/main/java/org/apache/sysds/hops/IndexingOp.java | 2 +-
src/main/java/org/apache/sysds/hops/LeftIndexingOp.java | 2 +-
src/main/java/org/apache/sysds/hops/LiteralOp.java | 2 +-
src/main/java/org/apache/sysds/hops/NaryOp.java | 2 +-
.../java/org/apache/sysds/hops/ParameterizedBuiltinOp.java | 2 +-
src/main/java/org/apache/sysds/hops/QuaternaryOp.java | 2 +-
src/main/java/org/apache/sysds/hops/ReorgOp.java | 2 +-
src/main/java/org/apache/sysds/hops/TernaryOp.java | 2 +-
src/main/java/org/apache/sysds/hops/UnaryOp.java | 2 +-
src/main/java/org/apache/sysds/hops/codegen/SpoofFusedOp.java | 2 +-
18 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/src/main/java/org/apache/sysds/hops/AggBinaryOp.java
b/src/main/java/org/apache/sysds/hops/AggBinaryOp.java
index 9b3356f..3b0da87 100644
--- a/src/main/java/org/apache/sysds/hops/AggBinaryOp.java
+++ b/src/main/java/org/apache/sysds/hops/AggBinaryOp.java
@@ -380,7 +380,7 @@ public class AggBinaryOp extends MultiThreadedHop
}
@Override
- protected ExecType optFindExecType()
+ protected ExecType optFindExecType(boolean transitive)
{
checkAndSetForcedPlatform();
@@ -407,7 +407,8 @@ public class AggBinaryOp extends MultiThreadedHop
}
//check for valid CP mmchain, send invalid memory
requirements to remote
- if( _etype == ExecType.CP && checkMapMultChain() !=
ChainType.NONE
+ if( _etype == ExecType.CP
+ && checkMapMultChain() != ChainType.NONE
&& OptimizerUtils.getLocalMemBudget() <
getInput().get(0).getInput().get(0).getOutputMemEstimate() )
_etype = ExecType.SPARK;
@@ -418,7 +419,7 @@ public class AggBinaryOp extends MultiThreadedHop
//spark-specific decision refinement (execute binary aggregate
w/ left or right spark input and
//single parent also in spark because it's likely cheap and
reduces data transfer)
- if( _etype == ExecType.CP && _etypeForced != ExecType.CP
+ if( transitive && _etype == ExecType.CP && _etypeForced !=
ExecType.CP
&& (isApplicableForTransitiveSparkExecType(true)
|| isApplicableForTransitiveSparkExecType(false)) )
{
diff --git a/src/main/java/org/apache/sysds/hops/AggUnaryOp.java
b/src/main/java/org/apache/sysds/hops/AggUnaryOp.java
index 424f730..2055411 100644
--- a/src/main/java/org/apache/sysds/hops/AggUnaryOp.java
+++ b/src/main/java/org/apache/sysds/hops/AggUnaryOp.java
@@ -339,7 +339,7 @@ public class AggUnaryOp extends MultiThreadedHop
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
@@ -373,13 +373,13 @@ public class AggUnaryOp extends MultiThreadedHop
//spark-specific decision refinement (execute unary aggregate
w/ spark input and
//single parent also in spark because it's likely cheap and
reduces data transfer)
//we also allow multiple parents, if all other parents are
already in Spark mode
- if( _etype == ExecType.CP && _etypeForced != ExecType.CP
+ if( transitive && _etype == ExecType.CP && _etypeForced !=
ExecType.CP
&& !(getInput().get(0) instanceof DataOp) //input is
not checkpoint
+ && getInput().get(0).optFindExecType() == ExecType.SPARK
&& (getInput().get(0).getParent().size()==1 //uagg is
only parent, or
||
getInput().get(0).getParent().stream().filter(h -> h != this)
- .allMatch(h -> h.optFindExecType() ==
ExecType.SPARK)
- || !requiresAggregation(getInput().get(0),
_direction)) //w/o agg
- && getInput().get(0).optFindExecType() ==
ExecType.SPARK )
+ .allMatch(h -> h.optFindExecType(false)
== ExecType.SPARK)
+ || !requiresAggregation(getInput().get(0),
_direction)) ) //w/o agg
{
//pull unary aggregate into spark
_etype = ExecType.SPARK;
diff --git a/src/main/java/org/apache/sysds/hops/BinaryOp.java
b/src/main/java/org/apache/sysds/hops/BinaryOp.java
index 8826f92..f3b9d0a 100644
--- a/src/main/java/org/apache/sysds/hops/BinaryOp.java
+++ b/src/main/java/org/apache/sysds/hops/BinaryOp.java
@@ -688,7 +688,7 @@ public class BinaryOp extends MultiThreadedHop
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
@@ -746,7 +746,7 @@ public class BinaryOp extends MultiThreadedHop
//spark-specific decision refinement (execute unary scalar w/
spark input and
//single parent also in spark because it's likely cheap and
reduces intermediates)
- if( _etype == ExecType.CP && _etypeForced != ExecType.CP
+ if( transitive && _etype == ExecType.CP && _etypeForced !=
ExecType.CP
&& getDataType().isMatrix() && (dt1.isScalar() ||
dt2.isScalar())
&& supportsMatrixScalarOperations()
//scalar operations
&& !(getInput().get(dt1.isScalar()?1:0) instanceof
DataOp) //input is not checkpoint
diff --git a/src/main/java/org/apache/sysds/hops/DataGenOp.java
b/src/main/java/org/apache/sysds/hops/DataGenOp.java
index 4ac5949..77a05cb 100644
--- a/src/main/java/org/apache/sysds/hops/DataGenOp.java
+++ b/src/main/java/org/apache/sysds/hops/DataGenOp.java
@@ -294,7 +294,7 @@ public class DataGenOp extends MultiThreadedHop
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
if( _etypeForced != null )
diff --git a/src/main/java/org/apache/sysds/hops/DataOp.java
b/src/main/java/org/apache/sysds/hops/DataOp.java
index 03dfd08..1d2237c 100644
--- a/src/main/java/org/apache/sysds/hops/DataOp.java
+++ b/src/main/java/org/apache/sysds/hops/DataOp.java
@@ -434,7 +434,7 @@ public class DataOp extends Hop {
}
@Override
- protected ExecType optFindExecType()
+ protected ExecType optFindExecType(boolean transitive)
{
//MB: find exec type has two meanings here: (1) for write it
means the actual
//exec type, while (2) for read it affects the recompilation
decision as needed
diff --git a/src/main/java/org/apache/sysds/hops/DnnOp.java
b/src/main/java/org/apache/sysds/hops/DnnOp.java
index 210d8b4..a5e3571 100644
--- a/src/main/java/org/apache/sysds/hops/DnnOp.java
+++ b/src/main/java/org/apache/sysds/hops/DnnOp.java
@@ -562,7 +562,7 @@ public class DnnOp extends MultiThreadedHop {
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/FunctionOp.java
b/src/main/java/org/apache/sysds/hops/FunctionOp.java
index 99c5bf0..367b845 100644
--- a/src/main/java/org/apache/sysds/hops/FunctionOp.java
+++ b/src/main/java/org/apache/sysds/hops/FunctionOp.java
@@ -310,7 +310,7 @@ public class FunctionOp extends Hop
}
@Override
- protected ExecType optFindExecType()
+ protected ExecType optFindExecType(boolean transitive)
{
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/Hop.java
b/src/main/java/org/apache/sysds/hops/Hop.java
index 0ef6d96..8808042 100644
--- a/src/main/java/org/apache/sysds/hops/Hop.java
+++ b/src/main/java/org/apache/sysds/hops/Hop.java
@@ -857,7 +857,11 @@ public abstract class Hop implements ParseInfo {
public abstract Lop constructLops();
- protected abstract ExecType optFindExecType();
+ protected final ExecType optFindExecType() {
+ return optFindExecType(true);
+ }
+
+ protected abstract ExecType optFindExecType(boolean transitive);
public abstract String getOpString();
diff --git a/src/main/java/org/apache/sysds/hops/IndexingOp.java
b/src/main/java/org/apache/sysds/hops/IndexingOp.java
index 30062fd..394a5bd 100644
--- a/src/main/java/org/apache/sysds/hops/IndexingOp.java
+++ b/src/main/java/org/apache/sysds/hops/IndexingOp.java
@@ -328,7 +328,7 @@ public class IndexingOp extends Hop
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/LeftIndexingOp.java
b/src/main/java/org/apache/sysds/hops/LeftIndexingOp.java
index 6be32d6..79d863a 100644
--- a/src/main/java/org/apache/sysds/hops/LeftIndexingOp.java
+++ b/src/main/java/org/apache/sysds/hops/LeftIndexingOp.java
@@ -289,7 +289,7 @@ public class LeftIndexingOp extends Hop
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/LiteralOp.java
b/src/main/java/org/apache/sysds/hops/LiteralOp.java
index 0716bd8..cd1d7aa 100644
--- a/src/main/java/org/apache/sysds/hops/LiteralOp.java
+++ b/src/main/java/org/apache/sysds/hops/LiteralOp.java
@@ -185,7 +185,7 @@ public class LiteralOp extends Hop
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
// Since a Literal hop does not represent any computation,
// this function is not applicable.
return null;
diff --git a/src/main/java/org/apache/sysds/hops/NaryOp.java
b/src/main/java/org/apache/sysds/hops/NaryOp.java
index bf2b286..e974d95 100644
--- a/src/main/java/org/apache/sysds/hops/NaryOp.java
+++ b/src/main/java/org/apache/sysds/hops/NaryOp.java
@@ -140,7 +140,7 @@ public class NaryOp extends Hop {
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/ParameterizedBuiltinOp.java
b/src/main/java/org/apache/sysds/hops/ParameterizedBuiltinOp.java
index 67a99de..b5be675 100644
--- a/src/main/java/org/apache/sysds/hops/ParameterizedBuiltinOp.java
+++ b/src/main/java/org/apache/sysds/hops/ParameterizedBuiltinOp.java
@@ -699,7 +699,7 @@ public class ParameterizedBuiltinOp extends
MultiThreadedHop {
}
@Override
- protected ExecType optFindExecType()
+ protected ExecType optFindExecType(boolean transitive)
{
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/QuaternaryOp.java
b/src/main/java/org/apache/sysds/hops/QuaternaryOp.java
index c882a2c..079cd33 100644
--- a/src/main/java/org/apache/sysds/hops/QuaternaryOp.java
+++ b/src/main/java/org/apache/sysds/hops/QuaternaryOp.java
@@ -729,7 +729,7 @@ public class QuaternaryOp extends MultiThreadedHop
}
@Override
- protected ExecType optFindExecType()
+ protected ExecType optFindExecType(boolean transitive)
{
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/ReorgOp.java
b/src/main/java/org/apache/sysds/hops/ReorgOp.java
index d2dcebf..64fd364 100644
--- a/src/main/java/org/apache/sysds/hops/ReorgOp.java
+++ b/src/main/java/org/apache/sysds/hops/ReorgOp.java
@@ -339,7 +339,7 @@ public class ReorgOp extends MultiThreadedHop
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/TernaryOp.java
b/src/main/java/org/apache/sysds/hops/TernaryOp.java
index 7d9fca6..07e8f45 100644
--- a/src/main/java/org/apache/sysds/hops/TernaryOp.java
+++ b/src/main/java/org/apache/sysds/hops/TernaryOp.java
@@ -477,7 +477,7 @@ public class TernaryOp extends MultiThreadedHop
@Override
- protected ExecType optFindExecType()
+ protected ExecType optFindExecType(boolean transitive)
{
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/UnaryOp.java
b/src/main/java/org/apache/sysds/hops/UnaryOp.java
index 71daf39..62acab7 100644
--- a/src/main/java/org/apache/sysds/hops/UnaryOp.java
+++ b/src/main/java/org/apache/sysds/hops/UnaryOp.java
@@ -460,7 +460,7 @@ public class UnaryOp extends MultiThreadedHop
}
@Override
- protected ExecType optFindExecType()
+ protected ExecType optFindExecType(boolean transitive)
{
checkAndSetForcedPlatform();
diff --git a/src/main/java/org/apache/sysds/hops/codegen/SpoofFusedOp.java
b/src/main/java/org/apache/sysds/hops/codegen/SpoofFusedOp.java
index be3ff1c..a6dcb19 100644
--- a/src/main/java/org/apache/sysds/hops/codegen/SpoofFusedOp.java
+++ b/src/main/java/org/apache/sysds/hops/codegen/SpoofFusedOp.java
@@ -143,7 +143,7 @@ public class SpoofFusedOp extends MultiThreadedHop
}
@Override
- protected ExecType optFindExecType() {
+ protected ExecType optFindExecType(boolean transitive) {
checkAndSetForcedPlatform();
