Baunsgaard commented on code in PR #1790:
URL: https://github.com/apache/systemds/pull/1790#discussion_r1142682665


##########
src/main/java/org/apache/sysds/hops/rewrite/RewriteInjectSparkPReadCheckpointing.java:
##########
@@ -68,20 +69,30 @@ private void rInjectCheckpointAfterPRead( Hop hop )
                boolean isMatrix = hop.getDataType().isMatrix();
                boolean isPRead = hop instanceof DataOp  && 
((DataOp)hop).getOp()==OpOpData.PERSISTENTREAD;
                boolean isFrameException = hop.getDataType().isFrame() && 
isPRead && !((DataOp)hop).getFileFormat().isIJV();
-               
-               if( (isMatrix && isPRead) || (hop.requiresReblock() && 
!isFrameException) ) {
+
+               // if the only operation performed is an action then do not add 
chkpoint
+               if((isMatrix && isPRead) || (hop.requiresReblock() && 
!isFrameException)) {
+                       boolean isActionOnly = isActionOnly(hop,  
hop.getParent());
                        //make given hop for checkpointing (w/ default storage 
level)
-                       //note: we do not recursively process childs here in 
order to prevent unnecessary checkpoints
-                       hop.setRequiresCheckpoint(true);
+                       //note: we do not recursively process children here in 
order to prevent unnecessary checkpoints
+                       if(!isActionOnly)
+                               hop.setRequiresCheckpoint(true);
                }
                else {
                        if( hop.getInput() != null ) {
-                               //process all childs (prevent concurrent 
modification by index access)
+                               //process all children (prevent concurrent 
modification by index access)
                                for( int i=0; i<hop.getInput().size(); i++ )
                                        rInjectCheckpointAfterPRead( 
hop.getInput().get(i) );
                        }
                }
                
                hop.setVisited();
        }
+
+       private boolean isActionOnly(Hop hop, List<Hop> parents){
+               // if the number of consumers of this hop is equal to 1 and no 
more
+               // then do not cache block unless that one operation is 
transient write
+               boolean isPWrite = hop instanceof DataOp && 
((DataOp)hop).getOp()==OpOpData.PERSISTENTWRITE;
+               return !isPWrite && parents.size() == 1;
+       }

Review Comment:
   thanks,



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@systemds.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to