[GR-2798] Small RFFI perfm optimizations and fix in StringVectorDataPtr.

steve-s · steve-s · commit 44cee348b6a5 · 2020-06-26T13:12:26.000Z
PullRequest: fastr/2481
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/AbstractContainerLibrary.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/AbstractContainerLibrary.java
@@ -71,6 +71,8 @@ public RAbstractVector createEmptySameType(Object container, @SuppressWarnings("
      */
     public abstract RAbstractContainer materialize(Object container);
 
+    public abstract RAbstractContainer cachedMaterialize(Object container);
+
     /**
      * Transfers this vector to be backed by native off heap memory. The data object should then be
      * able to respond to {@link VectorDataLibrary#asPointer(Object)}.
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RComplexVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RComplexVector.java
@@ -31,24 +31,23 @@
 import com.oracle.truffle.api.library.ExportLibrary;
 import com.oracle.truffle.api.library.ExportMessage;
 import com.oracle.truffle.api.library.ExportMessage.Ignore;
-
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.RType;
 import com.oracle.truffle.r.runtime.Utils;
+import com.oracle.truffle.r.runtime.data.RSharingAttributeStorage.Shareable;
+import com.oracle.truffle.r.runtime.data.closures.RClosure;
 import com.oracle.truffle.r.runtime.data.closures.RClosures;
+import com.oracle.truffle.r.runtime.data.model.RAbstractAtomicVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
 import com.oracle.truffle.r.runtime.data.model.RAbstractVector;
+import com.oracle.truffle.r.runtime.data.model.RAbstractVector.RMaterializedVector;
 import com.oracle.truffle.r.runtime.data.nodes.FastPathVectorAccess.FastPathFromComplexAccess;
 import com.oracle.truffle.r.runtime.data.nodes.SlowPathVectorAccess.SlowPathFromComplexAccess;
 import com.oracle.truffle.r.runtime.data.nodes.VectorAccess;
 import com.oracle.truffle.r.runtime.ops.na.NACheck;
-import com.oracle.truffle.r.runtime.data.RSharingAttributeStorage.Shareable;
-import com.oracle.truffle.r.runtime.data.closures.RClosure;
-import com.oracle.truffle.r.runtime.data.model.RAbstractAtomicVector;
-import com.oracle.truffle.r.runtime.data.model.RAbstractVector.RMaterializedVector;
+
 import java.util.Arrays;
-import java.util.concurrent.atomic.AtomicReference;
 
 @ExportLibrary(InteropLibrary.class)
 @ExportLibrary(AbstractContainerLibrary.class)
@@ -406,15 +405,6 @@ public long allocateNativeContents() {
         }
     }
 
-    private AtomicReference<RComplexVector> materialized = new AtomicReference<>();
-
-    public Object cachedMaterialize() {
-        if (materialized.get() == null) {
-            materialized.compareAndSet(null, materialize());
-        }
-        return materialized.get();
-    }
-
     private static final class FastPathAccess extends FastPathFromComplexAccess {
 
         FastPathAccess(RAbstractContainer value) {
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDoubleVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDoubleVector.java
@@ -45,7 +45,6 @@
 import com.oracle.truffle.r.runtime.data.model.RAbstractVector.RMaterializedVector;
 import com.oracle.truffle.r.runtime.data.nodes.FastPathVectorAccess.FastPathFromDoubleAccess;
 import java.util.Arrays;
-import java.util.concurrent.atomic.AtomicReference;
 
 @ExportLibrary(InteropLibrary.class)
 @ExportLibrary(AbstractContainerLibrary.class)
@@ -371,15 +370,6 @@ public long allocateNativeContents() {
         }
     }
 
-    private AtomicReference<RDoubleVector> materialized = new AtomicReference<>();
-
-    public Object cachedMaterialize() {
-        if (materialized.get() == null) {
-            materialized.compareAndSet(null, materialize());
-        }
-        return materialized.get();
-    }
-
     private static final class FastPathAccess extends FastPathFromDoubleAccess {
         FastPathAccess(RAbstractContainer value) {
             super(value);
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RIntVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RIntVector.java
@@ -43,7 +43,6 @@
 import com.oracle.truffle.r.runtime.data.nodes.VectorAccess;
 import com.oracle.truffle.r.runtime.ops.na.NACheck;
 import java.util.Arrays;
-import java.util.concurrent.atomic.AtomicReference;
 
 @ExportLibrary(InteropLibrary.class)
 @ExportLibrary(AbstractContainerLibrary.class)
@@ -388,15 +387,6 @@ public long allocateNativeContents() {
         }
     }
 
-    private final AtomicReference<RIntVector> materialized = new AtomicReference<>();
-
-    public Object cachedMaterialize() {
-        if (materialized.get() == null) {
-            materialized.compareAndSet(null, materialize());
-        }
-        return materialized.get();
-    }
-
     private static final class FastPathAccess extends FastPathFromIntAccess {
         FastPathAccess(RAbstractContainer value) {
             super(value);
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RLogicalVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RLogicalVector.java
@@ -47,7 +47,6 @@
 import com.oracle.truffle.r.runtime.data.model.RAbstractAtomicVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractVector.RMaterializedVector;
 import java.util.Arrays;
-import java.util.concurrent.atomic.AtomicReference;
 
 @ExportLibrary(InteropLibrary.class)
 @ExportLibrary(AbstractContainerLibrary.class)
@@ -385,15 +384,6 @@ public long allocateNativeContents() {
         }
     }
 
-    private final AtomicReference<RLogicalVector> materialized = new AtomicReference<>();
-
-    public Object cachedMaterialize() {
-        if (materialized.get() == null) {
-            materialized.compareAndSet(null, materialize());
-        }
-        return materialized.get();
-    }
-
     private static final class FastPathAccess extends FastPathFromLogicalAccess {
 
         FastPathAccess(RAbstractContainer value) {
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RRawVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RRawVector.java
@@ -43,7 +43,6 @@
 import com.oracle.truffle.r.runtime.data.model.RAbstractNumericVector;
 import com.oracle.truffle.r.runtime.data.model.RAbstractVector.RMaterializedVector;
 import java.util.Arrays;
-import java.util.concurrent.atomic.AtomicReference;
 
 @ExportLibrary(InteropLibrary.class)
 @ExportLibrary(AbstractContainerLibrary.class)
@@ -307,15 +306,6 @@ public long allocateNativeContents() {
         }
     }
 
-    private AtomicReference<RRawVector> materialized = new AtomicReference<>();
-
-    public Object cachedMaterialize() {
-        if (materialized.get() == null) {
-            materialized.compareAndSet(null, materialize());
-        }
-        return materialized.get();
-    }
-
     private static final class FastPathAccess extends FastPathFromRawAccess {
 
         FastPathAccess(RAbstractContainer value) {
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RStringVector.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RStringVector.java
@@ -49,8 +49,6 @@
 import com.oracle.truffle.r.runtime.ops.na.NACheck;
 import java.util.Arrays;
 
-import java.util.concurrent.atomic.AtomicReference;
-
 @ExportLibrary(InteropLibrary.class)
 @ExportLibrary(AbstractContainerLibrary.class)
 public final class RStringVector extends RAbstractAtomicVector implements RMaterializedVector, Shareable {
@@ -360,15 +358,6 @@ RStringVector containerLibCopy(@CachedLibrary(limit = DATA_LIB_LIMIT) VectorData
         return result;
     }
 
-    private AtomicReference<RStringVector> materialized = new AtomicReference<>();
-
-    public Object cachedMaterialize() {
-        if (materialized.get() == null) {
-            materialized.compareAndSet(null, materialize());
-        }
-        return materialized.get();
-    }
-
     @Override
     @Ignore // VectorDataLibrary
     public Object getDataAtAsObject(int index) {
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/model/RAbstractContainer.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/model/RAbstractContainer.java
@@ -43,6 +43,8 @@
 import com.oracle.truffle.r.runtime.data.VectorDataLibrary;
 import com.oracle.truffle.r.runtime.data.nodes.VectorAccess;
 
+import java.util.concurrent.atomic.AtomicReference;
+
 @ExportLibrary(AbstractContainerLibrary.class)
 public abstract class RAbstractContainer extends RSharingAttributeStorage {
 
@@ -194,6 +196,16 @@ public boolean containerLibIsComplete(@CachedLibrary(limit = DATA_LIB_LIMIT) Vec
         return dataLib.isComplete(data);
     }
 
+    private final AtomicReference<RAbstractContainer> materialized = new AtomicReference<>();
+
+    @ExportMessage(name = "cachedMaterialize", library = AbstractContainerLibrary.class)
+    public RAbstractContainer containerLibCachedMaterialize(@CachedLibrary("this") AbstractContainerLibrary containerLibrary) {
+        if (materialized.get() == null) {
+            materialized.compareAndSet(null, containerLibrary.materialize(this));
+        }
+        return materialized.get();
+    }
+
     @ExportMessage(name = "materialize", library = AbstractContainerLibrary.class)
     public RAbstractContainer containerLibMaterialize() {
         return materialize();
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/FFIMaterializeNode.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/FFIMaterializeNode.java
@@ -25,25 +25,24 @@
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.dsl.Fallback;
 import com.oracle.truffle.api.dsl.GenerateUncached;
+import com.oracle.truffle.api.dsl.ImportStatic;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.interop.TruffleObject;
+import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.r.runtime.DSLConfig;
 import com.oracle.truffle.r.runtime.RInternalError;
 import com.oracle.truffle.r.runtime.RRuntime;
 import com.oracle.truffle.r.runtime.context.RContext;
+import com.oracle.truffle.r.runtime.data.AbstractContainerLibrary;
 import com.oracle.truffle.r.runtime.data.NativeDataAccess;
 import com.oracle.truffle.r.runtime.data.RBaseObject;
 import com.oracle.truffle.r.runtime.data.RComplex;
-import com.oracle.truffle.r.runtime.data.RComplexVector;
 import com.oracle.truffle.r.runtime.data.RDataFactory;
-import com.oracle.truffle.r.runtime.data.RDoubleVector;
-import com.oracle.truffle.r.runtime.data.RIntVector;
-import com.oracle.truffle.r.runtime.data.RLogicalVector;
 import com.oracle.truffle.r.runtime.data.RRaw;
-import com.oracle.truffle.r.runtime.data.RRawVector;
 import com.oracle.truffle.r.runtime.data.RScalarVector;
 import com.oracle.truffle.r.runtime.data.RSequence;
-import com.oracle.truffle.r.runtime.data.RStringVector;
+import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
 import com.oracle.truffle.r.runtime.ffi.interop.NativeDoubleArray;
 
 /**
@@ -54,6 +53,7 @@
  *
  * See documentation/dev/ffi.md for more details.
  */
+@ImportStatic(DSLConfig.class)
 @GenerateUncached
 public abstract class FFIMaterializeNode extends Node {
 
@@ -130,36 +130,11 @@ protected static Object wrap(RBaseObject value, @SuppressWarnings("unused") bool
         return value;
     }
 
-    @Specialization
-    protected static Object wrap(RIntVector value, @SuppressWarnings("unused") boolean protect) {
-        // TODO specialize only for sequences (and maybe some other)
-        return value.cachedMaterialize();
-    }
-
-    @Specialization
-    protected static Object wrap(RDoubleVector value, @SuppressWarnings("unused") boolean protect) {
+    @Specialization(limit = "getGenericDataLibraryCacheSize()")
+    protected static Object wrap(RAbstractContainer value, @SuppressWarnings("unused") boolean protect,
+                    @CachedLibrary("value") AbstractContainerLibrary containerLibrary) {
         // TODO specialize only for sequences (and maybe some other)
-        return value.cachedMaterialize();
-    }
-
-    @Specialization
-    protected static Object wrap(RRawVector value, @SuppressWarnings("unused") boolean protect) {
-        return value.cachedMaterialize();
-    }
-
-    @Specialization
-    protected static Object wrap(RLogicalVector value, @SuppressWarnings("unused") boolean protect) {
-        return value.cachedMaterialize();
-    }
-
-    @Specialization
-    protected static Object wrap(RComplexVector value, @SuppressWarnings("unused") boolean protect) {
-        return value.cachedMaterialize();
-    }
-
-    @Specialization
-    protected static Object wrap(RStringVector value, @SuppressWarnings("unused") boolean protect) {
-        return value.cachedMaterialize();
+        return containerLibrary.cachedMaterialize(value);
     }
 
     // Symbol holds the address as a field
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/FFIWrap.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/FFIWrap.java
@@ -23,8 +23,8 @@
 package com.oracle.truffle.r.runtime.ffi;
 
 import com.oracle.truffle.api.CompilerAsserts;
-import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
+import com.oracle.truffle.r.runtime.RInternalError;
 
 /**
  *
@@ -105,7 +105,9 @@ public Object[] wrap(Object[] args, FFIMaterializeNode[] ffiMateralizeNodes, FFI
 
         @Override
         public void close() {
-            CompilerDirectives.materialize(materialized);
+            if (materialized == null) {
+                throw RInternalError.shouldNotReachHere();
+            }
         }
     }
 
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/RObjectDataPtr.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/RObjectDataPtr.java
@@ -286,6 +286,40 @@ private StringVectorDataPtr(RStringVector vector) {
         protected Object getNativeTypeImpl(RFFIContext rffi) {
             return rffi.getSulongArrayType(42L);
         }
+
+        @ExportMessage
+        public Object readArrayElement(long index,
+                        @Cached FFIToNativeMirrorNode wrapperNode,
+                        @CachedLibrary(limit = "DATA_LIB_LIMIT") VectorDataLibrary dataLib) {
+            String result = dataLib.getStringAt(vectorData, (int) index);
+            Object wrappedInCharSXP = CharSXPWrapper.create(result);
+            return wrapperNode.execute(wrappedInCharSXP);
+        }
+
+        @ExportMessage
+        protected void writeArrayElement(long index, Object value,
+                        @Cached FFIUnwrapNode unwrapNode,
+                        @Cached FFIUnwrapString unwrapString,
+                        @Cached("createBinaryProfile()") ConditionProfile isWrappedProfile,
+                        @Cached("createBinaryProfile()") ConditionProfile isStringWrappedProfile,
+                        @Cached BranchProfile notMaterializedBranchProfile,
+                        @CachedLibrary(limit = "DATA_LIB_LIMIT") VectorDataLibrary dataLib,
+                        @CachedLibrary(limit = "DATA_LIB_LIMIT") AbstractContainerLibrary containerLib) {
+            Object unwrappedValue = value;
+            if (isWrappedProfile.profile(value instanceof NativeDataAccess.NativeMirror)) {
+                unwrappedValue = unwrapNode.execute(value);
+            }
+            if (isStringWrappedProfile.profile(unwrappedValue instanceof CharSXPWrapper)) {
+                unwrappedValue = unwrapString.execute(unwrappedValue);
+            }
+            if (!dataLib.isWriteable(vectorData)) {
+                notMaterializedBranchProfile.enter();
+                containerLib.materializeData(vector);
+                vectorData = vector.getData();
+                assert dataLib.isWriteable(vectorData);
+            }
+            dataLib.setElementAt(vectorData, RRuntime.interopArrayIndexToInt(index, vector), unwrappedValue);
+        }
     }
 
     @ExportLibrary(InteropLibrary.class)

Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,8 @@`
`23`	`23`	`package com.oracle.truffle.r.runtime.ffi;`
`24`	`24`
`25`	`25`	`import com.oracle.truffle.api.CompilerAsserts;`
`26`		`-import com.oracle.truffle.api.CompilerDirectives;`
`27`	`26`	`import com.oracle.truffle.api.nodes.ExplodeLoop;`
	`27`	`+import com.oracle.truffle.r.runtime.RInternalError;`
`28`	`28`
`29`	`29`	`/**`
`30`	`30`	`*`
`@@ -105,7 +105,9 @@ public Object[] wrap(Object[] args, FFIMaterializeNode[] ffiMateralizeNodes, FFI`
`105`	`105`
`106`	`106`	`@Override`
`107`	`107`	`public void close() {`
`108`		`- CompilerDirectives.materialize(materialized);`
	`108`	`+ if (materialized == null) {`
	`109`	`+ throw RInternalError.shouldNotReachHere();`
	`110`	`+ }`
`109`	`111`	`}`
`110`	`112`	`}`
`111`	`113`