[GR-21134] Do not eagerly materialize vectors going to native code.

PullRequest: fastr/2482

Author: steve-s

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/VectorDataClosure.java

@@ -22,6 +22,7 @@
  */
 package com.oracle.truffle.r.runtime.data;
 
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.interop.TruffleObject;
 import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.library.ExportLibrary;
@@ -111,8 +112,18 @@ public Object materialize(@CachedLibrary("this.data") VectorDataLibrary dataLib)
         switch (getType()) {
             case Integer:
                 return new RIntArrayVectorData(getIntDataCopy(dataLib), dataLib.getNACheck(data).neverSeenNA());
+            case Double:
+                return new RDoubleArrayVectorData(getDoubleDataCopy(dataLib), dataLib.getNACheck(data).neverSeenNA());
+            case Raw:
+                return new RRawArrayVectorData(getRawDataCopy(dataLib));
+            case Logical:
+                return new RLogicalArrayVectorData(getLogicalDataCopy(dataLib), dataLib.getNACheck(data).neverSeenNA());
+            case Complex:
+                return new RComplexArrayVectorData(getComplexDataCopy(dataLib), dataLib.getNACheck(data).neverSeenNA());
+            case Character:
+                return new RStringArrayVectorData(getStringDataCopy(dataLib), dataLib.getNACheck(data).neverSeenNA());
             default:
-                throw RInternalError.unimplemented("TODO");
+                throw CompilerDirectives.shouldNotReachHere(getType().toString());
         }
     }
 

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/FFIMaterializeNode.java

@@ -28,28 +28,22 @@
 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.RDataFactory;
 import com.oracle.truffle.r.runtime.data.RRaw;
-import com.oracle.truffle.r.runtime.data.RScalarVector;
-import com.oracle.truffle.r.runtime.data.RSequence;
-import com.oracle.truffle.r.runtime.data.model.RAbstractContainer;
 import com.oracle.truffle.r.runtime.ffi.interop.NativeDoubleArray;
 
 /**
  * Converts values that can live on the FastR side to values that can be sent to the native code,
  * i.e., anything that is not {@link RBaseObject} and doesn't have a native mirror needs to be
- * wrapped to such object. Moreover, we need to convert compact read-only vectors to materialized
- * ones, since user may expect to be able to take their data-pointer and write into them.
+ * wrapped to such object.
  *
  * See documentation/dev/ffi.md for more details.
  */
@@ -58,101 +52,81 @@
 public abstract class FFIMaterializeNode extends Node {
 
     public Object materialize(Object value) {
-        return execute(value, false);
+        return execute(value);
     }
 
-    public Object materializeProtected(Object value) {
-        return execute(value, true);
-    }
-
-    protected abstract Object execute(Object value, boolean protect);
+    protected abstract Object execute(Object value);
 
     public static Object uncachedMaterialize(Object value) {
-        return FFIMaterializeNodeGen.getUncached().execute(value, false);
+        return FFIMaterializeNodeGen.getUncached().execute(value);
     }
 
     // Scalar values:
 
     @Specialization
-    protected static Object wrap(int value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(int value) {
         return RDataFactory.createIntVectorFromScalar(value);
     }
 
     @Specialization
-    protected static Object wrap(double value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(double value) {
         return RDataFactory.createDoubleVectorFromScalar(value);
     }
 
     @Specialization
-    protected static Object wrap(double[] value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(double[] value) {
         return new NativeDoubleArray(value);
     }
 
     @Specialization
-    protected static Object wrap(byte value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(byte value) {
         return RDataFactory.createLogicalVectorFromScalar(value);
     }
 
     @Specialization
-    protected static Object wrap(String value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(String value) {
         return RDataFactory.createStringVectorFromScalar(value);
     }
 
     @Specialization
-    protected static Object wrap(RRaw value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(RRaw value) {
         return RDataFactory.createRawVectorFromScalar(value);
     }
 
     @Specialization
-    protected static Object wrap(RComplex value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(RComplex value) {
         return RDataFactory.createComplexVectorFromScalar(value);
     }
 
-    // Sequences: life-cycle of the materialized vector is cached and tied with the sequence via a
-    // field inside the sequence
-
-    @Specialization
-    protected static Object wrap(RSequence seq, @SuppressWarnings("unused") boolean protect) {
-        return seq.cachedMaterialize();
-    }
-
     // RObjectDataPtr: held by a field in NativeMirror of the corresponding vector
 
     @Specialization
-    protected static Object wrap(RObjectDataPtr value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(RObjectDataPtr value) {
         return value;
     }
 
-    // No need to wrap other RObjects than sequences or scalars
-
-    @Specialization(guards = "!isRScalarVectorOrSequenceOrHasVectorData(value)")
-    protected static Object wrap(RBaseObject value, @SuppressWarnings("unused") boolean protect) {
+    // No need to wrap any RObjects
+    @Specialization
+    protected static Object wrap(RBaseObject value) {
         return value;
     }
 
-    @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 containerLibrary.cachedMaterialize(value);
-    }
-
     // Symbol holds the address as a field
 
     @Specialization
-    protected static Object wrap(DLL.SymbolHandle sym, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(DLL.SymbolHandle sym) {
         return sym.asAddress();
     }
 
     // Wrappers for foreign objects are held in a weak hash map
 
     @Specialization(guards = "isForeignObject(value)")
-    protected static Object wrapForeignObject(TruffleObject value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrapForeignObject(TruffleObject value) {
         return RContext.getInstance().getRFFI().getOrCreateForeignObjectWrapper(value);
     }
 
     @Fallback
-    protected static Object wrap(Object value, @SuppressWarnings("unused") boolean protect) {
+    protected static Object wrap(Object value) {
         CompilerDirectives.transferToInterpreter();
         String name = value == null ? "null" : value.getClass().getSimpleName();
         throw RInternalError.shouldNotReachHere("invalid wrapping: " + name);
@@ -164,10 +138,6 @@ static boolean isForeignObject(Object value) {
         return RRuntime.isForeignObject(value);
     }
 
-    protected static boolean isRScalarVectorOrSequenceOrHasVectorData(RBaseObject value) {
-        return value instanceof RScalarVector || value instanceof RSequence || RRuntime.hasVectorData(value);
-    }
-
     public static FFIMaterializeNode create() {
         return FFIMaterializeNodeGen.create();
     }

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/MiscRFFI.java

@@ -151,7 +151,7 @@ private JavaGDResizeCallNode(DownCallNodeFactory parent) {
         }
 
         public Object execute(VirtualFrame frame, Object dev) {
-            return call(frame, NativeFunction.javaGDresizeCall, opToNativeMirrorNode.execute(materializeNode.execute(dev, false)));
+            return call(frame, NativeFunction.javaGDresizeCall, opToNativeMirrorNode.execute(materializeNode.materialize(dev)));
         }
 
     }

com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/ffi/RObjectDataPtr.java

@@ -415,7 +415,7 @@ public Object readArrayElement(long index,
                         @Exclusive @Cached("createBinaryProfile()") ConditionProfile wasMaterializedProfile,
                         @CachedLibrary(limit = "DATA_LIB_LIMIT") VectorDataLibrary dataLib) {
             Object result = super.readArrayElement(index, dataLib);
-            Object materialized = materializeNode.execute(result, false);
+            Object materialized = materializeNode.materialize(result);
             if (wasMaterializedProfile.profile(materialized != result)) {
                 dataLib.setElementAt(vectorData, RRuntime.interopArrayIndexToInt(index, vector), materialized);
             }

com.oracle.truffle.r.test.native/packages/testrffi/testrffi/tests/simpleTests.R

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -407,3 +407,7 @@ if (Sys.getenv("TESTRFFI_IGNORE_4GB_VECTOR_TEST") != "") {
 }
 
 is.null(rffi.testMissingArgWithATTRIB())
+
+# Compact representations and RFFI:
+# sequences get materialized on write, but should not get materialized on read
+rffi.shareIntElement(1:2,1:3,1:4,1:5)