Merge pull request #1219 from p00ya/string-move-semantics

Avoid copies in String

Author: eddelbuettel

ChangeLog

@@ -1,3 +1,15 @@
+2022-05-25  Dirk Eddelbuettel  <[email protected]>
+
+	* inst/tinytest/test_string.R: Add C++11 conditioning for new unit tests
+
+2022-05-23  Dean Scarff  <[email protected]>
+
+	* inst/include/Rcpp/String.h: Make less copies of strings via move
+	semantics and preserving the buffer/SEXP representation when copying
+	* inst/include/RcppCommon.h: include <utility> for std::move
+	* inst/tinytest/cpp/String.cpp: Add unit tests
+	* inst/tinytest/test_string.R: Add unit tests
+
 2022-05-12  Dirk Eddelbuettel  <[email protected]>
 
 	* docker/ci/Dockerfile: Added xml2, also use more r-cran-* binaries

inst/include/Rcpp/String.h

@@ -58,8 +58,11 @@ namespace Rcpp {
         }
 
         /** copy constructor */
-        String(const String& other) : data(other.get_sexp()), token(R_NilValue), valid(true), buffer_ready(false), enc(Rf_getCharCE(other.get_sexp())) {
-            token = Rcpp_PreciousPreserve(data);
+        String(const String& s) : data(R_NilValue), token(R_NilValue), buffer(s.buffer), valid(s.valid), buffer_ready(s.buffer_ready), enc(s.enc) {
+            if (!buffer_ready) {
+                data = s.get_sexp();
+                token = Rcpp_PreciousPreserve(data);
+            }
             RCPP_STRING_DEBUG("String(const String&)");
         }
 
@@ -111,12 +114,27 @@ namespace Rcpp {
         }
 
         /** from a std::string */
-        String(const std::string& s, cetype_t enc = CE_UTF8) : buffer(s), valid(false), buffer_ready(true), enc(enc) {
-            data = R_NilValue;
-            token = R_NilValue;
+        String(const std::string& s, cetype_t enc = CE_UTF8) : data(R_NilValue), token(R_NilValue), buffer(s), valid(false), buffer_ready(true), enc(enc) {
             RCPP_STRING_DEBUG("String(const std::string&, cetype_t)");
         }
+#ifdef RCPP_USING_CXX11
+        /** move constructor */
+        String(String&& s) : data(s.data), token(s.token), buffer(std::move(s.buffer)), valid(s.valid), buffer_ready(s.buffer_ready), enc(s.enc) {
+            // Erase s.
+            s.data = R_NilValue;
+            s.token = R_NilValue;
+            s.buffer = std::string();
+            s.valid = false;
+            s.buffer_ready = true;
+            s.enc = CE_UTF8;
+            RCPP_STRING_DEBUG("String(String&&)");
+        }
 
+        /** move a std::string */
+        String(std::string&& s, cetype_t enc = CE_UTF8) : data(R_NilValue), token(R_NilValue), buffer(s), valid(false), buffer_ready(true), enc(enc) {
+            RCPP_STRING_DEBUG("String(std::string&&, cetype_t)");
+        }
+#endif
         String(const std::wstring& s, cetype_t enc = CE_UTF8) : data(internal::make_charsexp(s)), token(R_NilValue), valid(true), buffer_ready(false), enc(enc) {
             token = Rcpp_PreciousPreserve(data);
             RCPP_STRING_DEBUG("String(const std::wstring&, cetype_t)");
@@ -220,14 +238,27 @@ namespace Rcpp {
             return *this;
         }
         inline String& operator=(const String& other) {
-            SEXP x = other.get_sexp();
-            if (data != x) {
-                data = x;
-                Rcpp_PreciousRelease(token);
-                token = Rcpp_PreciousPreserve(x);
+            if (other.buffer_ready) {
+                // Copy the buffer without creating a SEXP.
+                if (valid) {
+                    Rcpp_PreciousRelease(token);
+                    valid = false;
+                }
+                data = R_NilValue;
+                token = R_NilValue;
+                buffer = other.buffer;
+                buffer_ready = true;
+                enc = other.enc;
+            } else {
+                SEXP x = other.get_sexp();
+                if (data != x) {
+                    data = x;
+                    Rcpp_PreciousRelease(token);
+                    token = Rcpp_PreciousPreserve(x);
+                }
+                valid = true;
+                buffer_ready = false;
             }
-            valid = true;
-            buffer_ready = false;
             return *this;
         }
         inline String& operator=(const std::string& s) {
@@ -236,6 +267,30 @@ namespace Rcpp {
             buffer_ready = true;
             return *this;
         }
+#ifdef RCPP_USING_CXX11
+        inline String& operator=(String&& other) {
+            data = other.data;
+            token = other.token;
+            buffer = std::move(other.buffer);
+            valid = other.valid;
+            buffer_ready = other.buffer_ready;
+            enc = other.enc;
+            // Erase other.
+            other.data = R_NilValue;
+            other.token = R_NilValue;
+            other.buffer = std::string();
+            other.valid = false;
+            other.buffer_ready = true;
+            other.enc = CE_UTF8;
+            return *this;
+        }
+        inline String& operator=(std::string&& s) {
+          buffer = s;
+          valid = false;
+          buffer_ready = true;
+          return *this;
+        }
+#endif
         inline String& operator=(const char* s) {
             buffer = s;
             valid = false;

inst/include/RcppCommon.h

@@ -66,6 +66,7 @@ namespace Rcpp {
 #include <cfloat>
 #include <limits>
 #include <typeinfo>
+#include <utility>
 #include <Rcpp/sprintf.h>
 #include <R_ext/Callbacks.h>
 #include <R_ext/Visibility.h>

inst/tinytest/cpp/String.cpp

@@ -53,6 +53,35 @@ String test_ctor(String x) {
     return test;
 }
 
+// [[Rcpp::export]]
+CharacterVector test_move_ctor() {
+  std::vector<String> v = { "test" };
+  String t = std::move( v[0] );
+  v.push_back( std::move( t ) );
+  return CharacterVector( v.begin(), v.end() );
+}
+
+// [[Rcpp::export]]
+String test_move_std_string_ctor() {
+  std::string s = "test";
+  return String( std::move( s ) );
+}
+
+// [[Rcpp::export]]
+CharacterVector test_move_assignment() {
+  std::vector<String> v = { "test", "abc" };
+  v[1] = std::move( v[0] );
+  return CharacterVector( v.begin(), v.end() );
+}
+
+// [[Rcpp::export]]
+String test_move_std_string_assignment() {
+  String x = "abc";
+  std::string s = "test";
+  x = std::move( s );
+  return x;
+}
+
 // [[Rcpp::export]]
 List test_compare_Strings( String aa, String bb ){
     return List::create(

inst/tinytest/test_string.R

@@ -1,5 +1,5 @@
 
-##  Copyright (C) 2012 - 2019  Dirk Eddelbuettel and Romain Francois
+##  Copyright (C) 2012 - 2022  Dirk Eddelbuettel and Romain Francois
 ##
 ##  This file is part of Rcpp.
 ##
@@ -63,6 +63,24 @@ expect_equal( res, target )
 res <- test_ctor("abc")
 expect_identical(res, "abc")
 
+if (Rcpp:::capabilities()[["Full C++11 support"]]) {
+    ##    test.String.move.ctor <- function() {
+    res <- test_move_ctor()
+    expect_identical(res, c("", "test"))
+
+    ##    test.String.move.std.string.ctor <- function() {
+    res <- test_move_std_string_ctor()
+    expect_identical(res, "test")
+
+    ##    test.String.move.assignment <- function() {
+    res <- test_move_assignment()
+    expect_identical(res, c("", "test"))
+
+    ##    test.String.move.std.string.assignment <- function() {
+    res <- test_move_std_string_assignment()
+    expect_identical(res, "test")
+}
+
 #    test.push.front <- function() {
 res <- test_push_front("def")
 expect_identical(res, "abcdef")