[systemds] branch master updated: [SYSTEMDS-2594] Fix bufferpool leak in mvvar instructions (for mice)

[mb...@apache.org]

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 bb197a6  [SYSTEMDS-2594] Fix bufferpool leak in mvvar instructions (for mice)
bb197a6 is described below

commit bb197a6c9556cb082d580282361319918ac04c7b
Author: Matthias Boehm <mb...@gmail.com>
AuthorDate: Sat Aug 1 01:01:55 2020 +0200

    [SYSTEMDS-2594] Fix bufferpool leak in mvvar instructions (for mice)
    
    A recent rework of the mice builtin in PR #972 made the existing
    BufferPoolLeak test fail again. The reason was an invalid cleanup logic
    in mvvar (move variable) instructions that only performed this cleanup
    for matrix and frame operands. However, mvvar instructions don't carry
    the data types and thus this cleanup never happened, leading to
    unnecessary evictions. This patch fixes the issue and also makes the
    entire mvvar logic slightly more efficient (less symbol table probes).
---
 .../instructions/cp/VariableCPInstruction.java       | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/main/java/org/apache/sysds/runtime/instructions/cp/VariableCPInstruction.java b/src/main/java/org/apache/sysds/runtime/instructions/cp/VariableCPInstruction.java
index 222b164..0e2b8ad 100644
--- a/src/main/java/org/apache/sysds/runtime/instructions/cp/VariableCPInstruction.java
+++ b/src/main/java/org/apache/sysds/runtime/instructions/cp/VariableCPInstruction.java
@@ -670,28 +670,24 @@ public class VariableCPInstruction extends CPInstruction implements LineageTrace
 	private void processMoveInstruction(ExecutionContext ec) {
 		
 		if ( getInput3() == null ) {
-			// example: mvvar tempA A
+			// example: mvvar tempA A (note that mvvar does not carry the data types)
 			
-			// get source variable 
-			Data srcData = ec.getVariable(getInput1().getName());
+			// get and remove source variable 
+			Data srcData = ec.removeVariable(getInput1().getName());
 			
 			if ( srcData == null ) {
 				throw new DMLRuntimeException("Unexpected error: could not find a data object "
 					+ "for variable name:" + getInput1().getName() + ", while processing instruction ");
 			}
 			
-			if( getInput2().getDataType().isMatrix() || getInput2().getDataType().isFrame() ) {
-				// remove existing variable bound to target name
-				Data tgt = ec.removeVariable(getInput2().getName());
-				
-				//cleanup matrix data on fs/hdfs (if necessary)
-				if( tgt != null )
-					ec.cleanupDataObject(tgt);
-			}
+			// remove existing variable bound to target name and
+			// cleanup matrix/frame/list data if necessary
+			Data tgt = ec.removeVariable(getInput2().getName());
+			if( tgt != null)
+				ec.cleanupDataObject(tgt);
 			
 			// do the actual move
 			ec.setVariable(getInput2().getName(), srcData);
-			ec.removeVariable(getInput1().getName());
 		}
 		else {
 			// example instruction: mvvar <srcVar> <destFile> <format>