This is an automated email from the ASF dual-hosted git repository.
mboehm7 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/main by this push:
new 4194823 [SYSTEMDS-3340] Fix index-out-of-bounds on reading large
dense blocks
4194823 is described below
commit 41948232c4e5d558d6bc5bccb807720e749e6527
Author: Matthias Boehm <[email protected]>
AuthorDate: Sun Mar 27 19:31:04 2022 +0200
[SYSTEMDS-3340] Fix index-out-of-bounds on reading large dense blocks
This patch fixes an issue of reading a large dense block (with multiple
double arrays) of shape 16 x 314660480, where every small 1k x 1k block
maps to multiple target blocks.
Besides a much cleaner implementation, this patch also improves the
performance for all FP64 combinations (small FP64 already had a special
implementation) via array copies of contiguous rows.
---
.../org/apache/sysds/runtime/data/DenseBlock.java | 50 +++++++++-------------
.../apache/sysds/runtime/data/DenseBlockBool.java | 6 +++
.../apache/sysds/runtime/data/DenseBlockFP32.java | 6 +++
.../apache/sysds/runtime/data/DenseBlockFP64.java | 6 +++
.../apache/sysds/runtime/data/DenseBlockInt32.java | 6 +++
.../apache/sysds/runtime/data/DenseBlockInt64.java | 6 +++
.../apache/sysds/runtime/data/DenseBlockLBool.java | 6 +++
.../apache/sysds/runtime/data/DenseBlockLFP32.java | 6 +++
.../apache/sysds/runtime/data/DenseBlockLFP64.java | 6 +++
.../sysds/runtime/data/DenseBlockLInt32.java | 6 +++
.../sysds/runtime/data/DenseBlockLInt64.java | 6 +++
.../sysds/runtime/data/DenseBlockLString.java | 6 +++
.../sysds/runtime/data/DenseBlockString.java | 6 +++
13 files changed, 92 insertions(+), 30 deletions(-)
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlock.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlock.java
index 53acc47..4e836e0 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlock.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlock.java
@@ -22,6 +22,7 @@ package org.apache.sysds.runtime.data;
import java.io.Serializable;
import java.util.Arrays;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.DMLRuntimeException;
import org.apache.sysds.runtime.instructions.cp.KahanObject;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -225,6 +226,13 @@ public abstract class DenseBlock implements Serializable
public abstract boolean isNumeric();
/**
+ * Indicates if the dense block is a specific numeric value type.
+ * @return true if numeric and of value type vt
+ */
+ public abstract boolean isNumeric(ValueType vt);
+
+
+ /**
* Indicates if the dense block has a single
* underlying block, i.e., if numBlocks==1.
*
@@ -472,41 +480,23 @@ public abstract class DenseBlock implements Serializable
* @return self
*/
public DenseBlock set(int rl, int ru, int cl, int cu, DenseBlock db) {
- // TODO: Optimize if dense block types match
- // TODO: Performance
boolean allColumns = cl == 0 && cu == _odims[0];
+ boolean FP64 = isNumeric(ValueType.FP64) &&
db.isNumeric(ValueType.FP64);
if (db.isNumeric()) {
int rowOther = 0;
- int colOther = 0;
for (int bi = index(rl); bi <= index(ru-1); bi++) {
- int rpos = bi*blockSize();
- int rposl = Math.max(rl-rpos, 0);
- if (allColumns) {
- int offset = rposl * _odims[0] + cl;
- int rlen =
Math.min(ru-Math.max(rpos,rl), blockSize(bi)-rposl);
- for (int i = 0; i < rlen * _odims[0];
i++) {
- setInternal(bi, offset + i,
db.get(rowOther, colOther));
- colOther++;
- if (colOther ==
db.getCumODims(0)) {
- rowOther++;
- colOther = 0;
- }
- }
+ int brl = Math.max(rl-bi*blockSize(), 0);
+ int bru = Math.min(ru-bi*blockSize(),
blockSize());
+ int offset = brl * _odims[0] + cl;
+ int clen = cu - cl;
+ for(int r = brl; r < bru; r++,
offset+=_odims[0], rowOther++) {
+ if( !FP64 )
+ for(int c = 0; c < clen; c++)
+ setInternal(bi, offset
+ c, db.get(rowOther, c));
+ else
+
System.arraycopy(db.values(rowOther),
+ db.pos(rowOther),
valuesAt(bi), offset, clen);
}
- else {
- int len = cu - cl;
- for (int i = rl, ix1 = rl * _odims[0] +
cl; i < ru; i++, ix1 += _odims[0]) {
- for (int ix = 0; ix < len;
ix++) {
- setInternal(bi, ix1 +
ix, db.get(rowOther, colOther));
- colOther++;
- if (colOther ==
db.getCumODims(0)) {
- rowOther++;
- colOther = 0;
- }
- }
- }
- }
- rl = 0;
}
}
else {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockBool.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockBool.java
index 2461aa8..0c46c36 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockBool.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockBool.java
@@ -23,6 +23,7 @@ package org.apache.sysds.runtime.data;
import java.util.BitSet;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -65,6 +66,11 @@ public class DenseBlockBool extends DenseBlockDRB
}
@Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.BOOLEAN == vt;
+ }
+
+ @Override
public void reset(int rlen, int[] odims, double v) {
boolean bv = v != 0;
int len = rlen * odims[0];
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP32.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP32.java
index d316110..1a3bc23 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP32.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP32.java
@@ -21,6 +21,7 @@ package org.apache.sysds.runtime.data;
import java.util.Arrays;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.common.Warnings;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -54,6 +55,11 @@ public class DenseBlockFP32 extends DenseBlockDRB
public boolean isNumeric() {
return true;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.FP32 == vt;
+ }
@Override
public long capacity() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP64.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP64.java
index fbb8286..916f315 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP64.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockFP64.java
@@ -20,6 +20,7 @@
package org.apache.sysds.runtime.data;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.UtilFunctions;
import org.apache.sysds.utils.MemoryEstimates;
@@ -52,6 +53,11 @@ public class DenseBlockFP64 extends DenseBlockDRB
}
@Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.FP64 == vt;
+ }
+
+ @Override
public void reset(int rlen, int[] odims, double v) {
int len = rlen * odims[0];
if( len > capacity() ) {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt32.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt32.java
index 304df44..47e6a81 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt32.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt32.java
@@ -22,6 +22,7 @@ package org.apache.sysds.runtime.data;
import java.util.Arrays;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -54,6 +55,11 @@ public class DenseBlockInt32 extends DenseBlockDRB
public boolean isNumeric() {
return true;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.INT32 == vt;
+ }
@Override
public long capacity() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt64.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt64.java
index 3c7d8d1..2be0370 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt64.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockInt64.java
@@ -22,6 +22,7 @@ package org.apache.sysds.runtime.data;
import java.util.Arrays;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -56,6 +57,11 @@ public class DenseBlockInt64 extends DenseBlockDRB
}
@Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.INT64 == vt;
+ }
+
+ @Override
public long capacity() {
return (_data!=null) ? _data.length : -1;
}
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLBool.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLBool.java
index b2cf3ad..705f894 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLBool.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLBool.java
@@ -21,6 +21,7 @@
package org.apache.sysds.runtime.data;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -57,6 +58,11 @@ public class DenseBlockLBool extends DenseBlockLDRB
public boolean isNumeric() {
return true;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.BOOLEAN == vt;
+ }
@Override
public boolean isContiguous() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP32.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP32.java
index 50ddc53..753c7a8 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP32.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP32.java
@@ -20,6 +20,7 @@
package org.apache.sysds.runtime.data;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -55,6 +56,11 @@ public class DenseBlockLFP32 extends DenseBlockLDRB
public boolean isNumeric() {
return true;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.FP32 == vt;
+ }
@Override
public boolean isContiguous() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP64.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP64.java
index 4634a96..2ffac04 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP64.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLFP64.java
@@ -19,6 +19,7 @@
package org.apache.sysds.runtime.data;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.UtilFunctions;
import java.util.Arrays;
@@ -53,6 +54,11 @@ public class DenseBlockLFP64 extends DenseBlockLDRB
public boolean isNumeric() {
return true;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.FP64 == vt;
+ }
@Override
public boolean isContiguous() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt32.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt32.java
index a0dc7f9..826b73b 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt32.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt32.java
@@ -21,6 +21,7 @@
package org.apache.sysds.runtime.data;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -56,6 +57,11 @@ public class DenseBlockLInt32 extends DenseBlockLDRB
public boolean isNumeric() {
return true;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.INT32 == vt;
+ }
@Override
public boolean isContiguous() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt64.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt64.java
index d173248..43228b1 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt64.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLInt64.java
@@ -21,6 +21,7 @@
package org.apache.sysds.runtime.data;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -56,6 +57,11 @@ public class DenseBlockLInt64 extends DenseBlockLDRB
public boolean isNumeric() {
return true;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return ValueType.INT64 == vt;
+ }
@Override
public boolean isContiguous() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLString.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLString.java
index 84ebfc6..191017a 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockLString.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockLString.java
@@ -21,6 +21,7 @@
package org.apache.sysds.runtime.data;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -56,6 +57,11 @@ public class DenseBlockLString extends DenseBlockLDRB
public boolean isNumeric() {
return false;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return false;
+ }
@Override
public boolean isContiguous() {
diff --git a/src/main/java/org/apache/sysds/runtime/data/DenseBlockString.java
b/src/main/java/org/apache/sysds/runtime/data/DenseBlockString.java
index 02ebd2a..2bba6a7 100644
--- a/src/main/java/org/apache/sysds/runtime/data/DenseBlockString.java
+++ b/src/main/java/org/apache/sysds/runtime/data/DenseBlockString.java
@@ -21,6 +21,7 @@
package org.apache.sysds.runtime.data;
import org.apache.sysds.common.Warnings;
+import org.apache.sysds.common.Types.ValueType;
import org.apache.sysds.runtime.util.DataConverter;
import org.apache.sysds.runtime.util.UtilFunctions;
@@ -54,6 +55,11 @@ public class DenseBlockString extends DenseBlockDRB {
public boolean isNumeric() {
return false;
}
+
+ @Override
+ public boolean isNumeric(ValueType vt) {
+ return false;
+ }
@Override
public long capacity() {
