From 71cd6a38fe539a89dc2ee159c6d54017fd4e90a0 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Sun, 3 Jul 2022 13:58:26 +0100 Subject: [PATCH] Performance optimisation for directors for classes passed by value The directorin typemaps in the director methods now use std::move on the input parameter when copying the object from the stack to the heap prior to the callback into the target language, thereby taking advantage of move semantics if available. --- CHANGES.current | 6 +++ .../csharp/director_pass_by_value_runme.cs | 2 + Examples/test-suite/director_pass_by_value.i | 41 +++++++++++++++++-- .../java/director_pass_by_value_runme.java | 2 + .../ocaml/director_pass_by_value_runme.ml | 3 ++ .../php/director_pass_by_value_runme.php | 3 ++ .../python/director_pass_by_value_runme.py | 2 + Lib/csharp/boost_shared_ptr.i | 2 +- Lib/csharp/csharp.swg | 2 +- Lib/d/boost_shared_ptr.i | 2 +- Lib/d/dswigtype.swg | 2 +- Lib/go/go.swg | 2 +- Lib/java/boost_shared_ptr.i | 2 +- Lib/java/java.swg | 2 +- Lib/java/swiginterface.i | 2 +- Lib/ocaml/ocaml.swg | 2 +- Lib/octave/boost_shared_ptr.i | 2 +- Lib/php/php.swg | 2 +- Lib/python/boost_shared_ptr.i | 2 +- Lib/r/boost_shared_ptr.i | 2 +- Lib/ruby/boost_shared_ptr.i | 2 +- Lib/scilab/boost_shared_ptr.i | 2 +- Lib/swig.swg | 9 ++++ Lib/typemaps/swigtype.swg | 2 +- 24 files changed, 80 insertions(+), 20 deletions(-) diff --git a/CHANGES.current b/CHANGES.current index 57dd6c765..9adfa8ad0 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,12 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.1.0 (in progress) =========================== +2022-07-03: wsfulton + Performane optimisation for directors for classes passed by value. The directorin + typemaps in the director methods now use std::move on the input parameter when + copying the object from the stack to the heap prior to the callback into the target + language, thereby taking advantage of move semantics if available. + 2022-07-02: wsfulton #1722 [C#, Java, Python, Ruby] Add std::unique_ptr support. Ported from std::auto_ptr. Use the %unique_ptr(T) macro as follows for usage std::unique_ptr. For example, for diff --git a/Examples/test-suite/csharp/director_pass_by_value_runme.cs b/Examples/test-suite/csharp/director_pass_by_value_runme.cs index ba6371590..1f64f6d7e 100644 --- a/Examples/test-suite/csharp/director_pass_by_value_runme.cs +++ b/Examples/test-suite/csharp/director_pass_by_value_runme.cs @@ -28,6 +28,8 @@ public class runme break; }; } + if (director_pass_by_value.has_cplusplus11()) + Counter.check_counts(1, 0, 0, 1, 0, 1); // check move constructor called and just one destructor // bug was the passByVal 'global' object was destroyed after the call to virtualMethod had finished. int ret = runme.passByVal.getVal(); if (ret != 0x12345678) diff --git a/Examples/test-suite/director_pass_by_value.i b/Examples/test-suite/director_pass_by_value.i index 31d8ce2d2..5e31c9d3b 100644 --- a/Examples/test-suite/director_pass_by_value.i +++ b/Examples/test-suite/director_pass_by_value.i @@ -1,14 +1,38 @@ %module(directors="1") director_pass_by_value + +#if defined(SWIGD) +%rename(trace) debug; +#endif + %director DirectorPassByValueAbstractBase; +%include "cpp11_move_only_helper.i" + +%ignore PassedByValue::operator=; +%ignore PassedByValue::PassedByValue(PassedByValue &&); + %inline %{ -class PassedByValue { - int val; -public: - PassedByValue() { val = 0x12345678; } +#include +using namespace std; +int debug = false; +struct PassedByValue { + PassedByValue(int v = 0x12345678) { val = v; if (debug) cout << "PassedByValue(0x" << hex << val << ")" << " " << this << endl; Counter::normal_constructor++; } + + PassedByValue(const PassedByValue &other) { val = other.val; if (debug) cout << "PassedByValue(const PassedByValue &)" << " " << this << " " << &other << endl; Counter::copy_constructor++;} + PassedByValue & operator=(const PassedByValue &other) { val = other.val; if (debug) cout << "operator=(const PassedByValue &)" << " " << this << " " << &other << endl; Counter::copy_assignment++; return *this; } + +#if __cplusplus >= 201103L + PassedByValue(PassedByValue &&other) noexcept { val = other.val; if (debug) cout << "PassedByValue(PassedByValue &&)" << " " << this << endl; Counter::move_constructor++; } + PassedByValue & operator=(PassedByValue &&other) noexcept { val = other.val; if (debug) cout << "operator=(PassedByValue &&)" << " " << this << endl; Counter::move_assignment++; return *this; } + ~PassedByValue() { if (debug) cout << "~PassedByValue()" << " " << this << endl; Counter::destructor++; } +#endif + int getVal() { return val; } +private: + int val; }; + int doSomething(int x) { int yy[256]; yy[0] =0x9876; @@ -18,6 +42,7 @@ int doSomething(int x) { class DirectorPassByValueAbstractBase { public: virtual void virtualMethod(PassedByValue pbv) = 0; + virtual void virtualConstMethod(const PassedByValue pbv) {} virtual ~DirectorPassByValueAbstractBase () {} }; @@ -27,4 +52,12 @@ public: f.virtualMethod(PassedByValue()); } }; + +bool has_cplusplus11() { +#if __cplusplus >= 201103L + return true; +#else + return false; +#endif +} %} diff --git a/Examples/test-suite/java/director_pass_by_value_runme.java b/Examples/test-suite/java/director_pass_by_value_runme.java index 24ded2ccf..1d34c3b55 100644 --- a/Examples/test-suite/java/director_pass_by_value_runme.java +++ b/Examples/test-suite/java/director_pass_by_value_runme.java @@ -32,6 +32,8 @@ public class director_pass_by_value_runme { break; }; } + if (director_pass_by_value.has_cplusplus11()) + Counter.check_counts(1, 0, 0, 1, 0, 1); // check move constructor called and just one destructor // bug was the passByVal 'global' object was destroyed after the call to virtualMethod had finished. int ret = director_pass_by_value_runme.passByVal.getVal(); if (ret != 0x12345678) diff --git a/Examples/test-suite/ocaml/director_pass_by_value_runme.ml b/Examples/test-suite/ocaml/director_pass_by_value_runme.ml index af862f189..ce2137fbe 100644 --- a/Examples/test-suite/ocaml/director_pass_by_value_runme.ml +++ b/Examples/test-suite/ocaml/director_pass_by_value_runme.ml @@ -14,10 +14,13 @@ let d = (director_pass_by_value_Derived) '() +let cpp11 = _has_cplusplus11 '() as bool + let _ = let caller = new_Caller '() in assert (caller -> call_virtualMethod (d) = C_void); assert (Array.length !passByVal = 1); +(* TODO: only run if cpp11... let _ = _Counter_check_counts (C_list [C_int 0; C_int 0; C_int 0; C_int 1; C_int 0; C_int 1]) in*) (* check move constructor called and just one destructor *) let a = List.hd (fnhelper (!passByVal.(0))) in assert (a -> getVal () as int = 0x12345678); assert (a -> "~" () = C_void); diff --git a/Examples/test-suite/php/director_pass_by_value_runme.php b/Examples/test-suite/php/director_pass_by_value_runme.php index d3763292f..b8f1bb9f9 100644 --- a/Examples/test-suite/php/director_pass_by_value_runme.php +++ b/Examples/test-suite/php/director_pass_by_value_runme.php @@ -14,6 +14,9 @@ class director_pass_by_value_Derived extends DirectorPassByValueAbstractBase { # bug was the passByVal global object was destroyed after the call to virtualMethod had finished. $caller = new Caller(); $caller->call_virtualMethod(new director_pass_by_value_Derived()); +if (has_cplusplus11()) { + Counter::check_counts(1, 0, 0, 1, 0, 1); # check move constructor called and just one destructor +} $ret = $passByVal->getVal(); if ($ret != 0x12345678) { check::fail("Bad return value, got " . dechex($ret)); diff --git a/Examples/test-suite/python/director_pass_by_value_runme.py b/Examples/test-suite/python/director_pass_by_value_runme.py index 7744db962..9dbd64ad6 100644 --- a/Examples/test-suite/python/director_pass_by_value_runme.py +++ b/Examples/test-suite/python/director_pass_by_value_runme.py @@ -8,6 +8,8 @@ class director_pass_by_value_Derived(director_pass_by_value.DirectorPassByValueA # bug was the passByVal global object was destroyed after the call to virtualMethod had finished. director_pass_by_value.Caller().call_virtualMethod(director_pass_by_value_Derived()) +if director_pass_by_value.has_cplusplus11(): + director_pass_by_value.Counter.check_counts(1, 0, 0, 1, 0, 1) # check move constructor called and just one destructor ret = passByVal.getVal(); if ret != 0x12345678: raise RuntimeError("Bad return value, got " + hex(ret)) diff --git a/Lib/csharp/boost_shared_ptr.i b/Lib/csharp/boost_shared_ptr.i index 508c0ec14..2bbb6e277 100644 --- a/Lib/csharp/boost_shared_ptr.i +++ b/Lib/csharp/boost_shared_ptr.i @@ -32,7 +32,7 @@ %{ $result = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(($1_ltype &)$1)); %} %typemap(directorin) CONST TYPE -%{ $input = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > (new $1_ltype((const $1_ltype &)$1)); %} +%{ $input = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > (new $1_ltype(SWIG_STD_MOVE($1))); %} %typemap(directorout) CONST TYPE %{ if (!$input) { diff --git a/Lib/csharp/csharp.swg b/Lib/csharp/csharp.swg index d4bcd3c80..94e0458a6 100644 --- a/Lib/csharp/csharp.swg +++ b/Lib/csharp/csharp.swg @@ -409,7 +409,7 @@ SWIGINTERN const char * SWIG_UnpackData(const char *c, void *ptr, size_t sz) { #endif %typemap(directorin) SWIGTYPE -%{ $input = (void *)new $1_ltype((const $1_ltype &)$1); %} +%{ $input = (void *)new $1_ltype(SWIG_STD_MOVE($1)); %} %typemap(csdirectorin) SWIGTYPE "new $&csclassname($iminput, true)" %typemap(csdirectorout) SWIGTYPE "$&csclassname.getCPtr($cscall).Handle" diff --git a/Lib/d/boost_shared_ptr.i b/Lib/d/boost_shared_ptr.i index 4a220a589..1018bd1f2 100644 --- a/Lib/d/boost_shared_ptr.i +++ b/Lib/d/boost_shared_ptr.i @@ -26,7 +26,7 @@ %{ $result = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(($1_ltype &)$1)); %} %typemap(directorin) CONST TYPE -%{ $input = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > (new $1_ltype((const $1_ltype &)$1)); %} +%{ $input = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > (new $1_ltype(SWIG_STD_MOVE($1))); %} %typemap(directorout) CONST TYPE %{ if (!$input) { diff --git a/Lib/d/dswigtype.swg b/Lib/d/dswigtype.swg index 535e8d576..3fb7f6a01 100644 --- a/Lib/d/dswigtype.swg +++ b/Lib/d/dswigtype.swg @@ -62,7 +62,7 @@ #endif %typemap(directorin) SWIGTYPE - "$input = (void *)new $1_ltype((const $1_ltype &)$1);" + "$input = (void *)new $1_ltype(SWIG_STD_MOVE($1));" %typemap(directorout) SWIGTYPE %{ if (!$input) { SWIG_DSetPendingException(SWIG_DIllegalArgumentException, "Unexpected null return for type $1_type"); diff --git a/Lib/go/go.swg b/Lib/go/go.swg index 6e415d2c1..bb7a471cc 100644 --- a/Lib/go/go.swg +++ b/Lib/go/go.swg @@ -625,7 +625,7 @@ %typemap(goout) SWIGTYPE "" %typemap(directorin) SWIGTYPE -%{ $input = new $1_ltype((const $1_ltype &)$1); %} +%{ $input = new $1_ltype(SWIG_STD_MOVE($1)); %} %typemap(godirectorin) SWIGTYPE "" diff --git a/Lib/java/boost_shared_ptr.i b/Lib/java/boost_shared_ptr.i index 325a6832d..adf9c203e 100644 --- a/Lib/java/boost_shared_ptr.i +++ b/Lib/java/boost_shared_ptr.i @@ -33,7 +33,7 @@ %typemap(directorin,descriptor="L$packagepath/$&javaclassname;") CONST TYPE %{ $input = 0; - *((SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > **)&$input) = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > (new $1_ltype((const $1_ltype &)$1)); %} + *((SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > **)&$input) = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > (new $1_ltype(SWIG_STD_MOVE($1))); %} %typemap(directorout) CONST TYPE %{ if (!$input) { diff --git a/Lib/java/java.swg b/Lib/java/java.swg index 7967c1882..23744aeb3 100644 --- a/Lib/java/java.swg +++ b/Lib/java/java.swg @@ -676,7 +676,7 @@ Swig::LocalRefGuard $1_refguard(jenv, $input); } %typemap(directorin,descriptor="L$packagepath/$&javaclassname;") SWIGTYPE %{ $input = 0; - *(($&1_ltype*)&$input) = new $1_ltype((const $1_ltype &)$1); %} + *(($&1_ltype*)&$input) = new $1_ltype(SWIG_STD_MOVE($1)); %} %typemap(javadirectorin) SWIGTYPE "new $&javaclassname($jniinput, true)" %typemap(javadirectorout) SWIGTYPE "$&javaclassname.getCPtr($javacall)" diff --git a/Lib/java/swiginterface.i b/Lib/java/swiginterface.i index 0a0f7806a..c3ca97d36 100644 --- a/Lib/java/swiginterface.i +++ b/Lib/java/swiginterface.i @@ -40,7 +40,7 @@ %typemap(javadirectorout) CTYPE *const& "$javacall.$*interfacename_GetInterfaceCPtr()" %typemap(directorin,descriptor="L$packagepath/$&javainterfacename;") CTYPE %{ $input = 0; - *(($&1_ltype*)&$input) = new $1_ltype((const $1_ltype &)$1); %} + *(($&1_ltype*)&$input) = new $1_ltype(SWIG_STD_MOVE($1)); %} %typemap(directorin,descriptor="L$packagepath/$javainterfacename;") CTYPE *, CTYPE [] %{ *(($&1_ltype)&$input) = ($1_ltype) $1; %} %typemap(directorin,descriptor="L$packagepath/$javainterfacename;") CTYPE & diff --git a/Lib/ocaml/ocaml.swg b/Lib/ocaml/ocaml.swg index 0c190bbc4..ef0a64c90 100644 --- a/Lib/ocaml/ocaml.swg +++ b/Lib/ocaml/ocaml.swg @@ -119,7 +119,7 @@ } %typemap(directorin) SWIGTYPE { - $<ype temp = new $ltype((const $ltype &)$1); + $<ype temp = new $1_ltype(SWIG_STD_MOVE($1)); swig_result = SWIG_Ocaml_ptr_to_val("create_$ltype_from_ptr", (void *)temp, $&1_descriptor); args = caml_list_append(args, swig_result); } diff --git a/Lib/octave/boost_shared_ptr.i b/Lib/octave/boost_shared_ptr.i index 668bf4354..a1427b74d 100644 --- a/Lib/octave/boost_shared_ptr.i +++ b/Lib/octave/boost_shared_ptr.i @@ -59,7 +59,7 @@ } %typemap(directorin,noblock=1) CONST TYPE (SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > *smartarg = 0) %{ - smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(($1_ltype &)$1)); + smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(SWIG_STD_MOVE($1))); $input = SWIG_NewPointerObj(%as_voidptr(smartarg), $descriptor(SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< TYPE > *), SWIG_POINTER_OWN | %newpointer_flags); %} %typemap(directorout,noblock=1) CONST TYPE (void *swig_argp, int swig_res = 0) { diff --git a/Lib/php/php.swg b/Lib/php/php.swg index 7d6d5f29d..c2442d24b 100644 --- a/Lib/php/php.swg +++ b/Lib/php/php.swg @@ -454,7 +454,7 @@ %typemap(directorin) SWIGTYPE %{ ZVAL_UNDEF($input); - SWIG_SetPointerZval($input, SWIG_as_voidptr(new $1_ltype((const $1_ltype &)$1)), $&1_descriptor, 1); + SWIG_SetPointerZval($input, SWIG_as_voidptr(new $1_ltype(SWIG_STD_MOVE($1))), $&1_descriptor, 1); %} %typemap(out, phptype="void") void ""; diff --git a/Lib/python/boost_shared_ptr.i b/Lib/python/boost_shared_ptr.i index 709e7811d..ab9962ac8 100644 --- a/Lib/python/boost_shared_ptr.i +++ b/Lib/python/boost_shared_ptr.i @@ -63,7 +63,7 @@ } %typemap(directorin,noblock=1) CONST TYPE (SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > *smartarg = 0) %{ - smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(($1_ltype &)$1)); + smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(SWIG_STD_MOVE($1))); $input = SWIG_NewPointerObj(%as_voidptr(smartarg), $descriptor(SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< TYPE > *), SWIG_POINTER_OWN | %newpointer_flags); %} %typemap(directorout,noblock=1) CONST TYPE (void *swig_argp, int swig_res = 0) { diff --git a/Lib/r/boost_shared_ptr.i b/Lib/r/boost_shared_ptr.i index 668bf4354..a1427b74d 100644 --- a/Lib/r/boost_shared_ptr.i +++ b/Lib/r/boost_shared_ptr.i @@ -59,7 +59,7 @@ } %typemap(directorin,noblock=1) CONST TYPE (SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > *smartarg = 0) %{ - smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(($1_ltype &)$1)); + smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(SWIG_STD_MOVE($1))); $input = SWIG_NewPointerObj(%as_voidptr(smartarg), $descriptor(SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< TYPE > *), SWIG_POINTER_OWN | %newpointer_flags); %} %typemap(directorout,noblock=1) CONST TYPE (void *swig_argp, int swig_res = 0) { diff --git a/Lib/ruby/boost_shared_ptr.i b/Lib/ruby/boost_shared_ptr.i index 9676bf9d8..fe3ea8cde 100644 --- a/Lib/ruby/boost_shared_ptr.i +++ b/Lib/ruby/boost_shared_ptr.i @@ -59,7 +59,7 @@ } %typemap(directorin,noblock=1) CONST TYPE (SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > *smartarg = 0) %{ - smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(($1_ltype &)$1)); + smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(SWIG_STD_MOVE($1))); $input = SWIG_NewPointerObj(%as_voidptr(smartarg), $descriptor(SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< TYPE > *), SWIG_POINTER_OWN | %newpointer_flags); %} %typemap(directorout,noblock=1) CONST TYPE (void *swig_argp, int swig_res = 0) { diff --git a/Lib/scilab/boost_shared_ptr.i b/Lib/scilab/boost_shared_ptr.i index 668bf4354..a1427b74d 100644 --- a/Lib/scilab/boost_shared_ptr.i +++ b/Lib/scilab/boost_shared_ptr.i @@ -59,7 +59,7 @@ } %typemap(directorin,noblock=1) CONST TYPE (SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE > *smartarg = 0) %{ - smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(($1_ltype &)$1)); + smartarg = new SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< CONST TYPE >(new $1_ltype(SWIG_STD_MOVE($1))); $input = SWIG_NewPointerObj(%as_voidptr(smartarg), $descriptor(SWIG_SHARED_PTR_QNAMESPACE::shared_ptr< TYPE > *), SWIG_POINTER_OWN | %newpointer_flags); %} %typemap(directorout,noblock=1) CONST TYPE (void *swig_argp, int swig_res = 0) { diff --git a/Lib/swig.swg b/Lib/swig.swg index ca7586457..c9b301426 100644 --- a/Lib/swig.swg +++ b/Lib/swig.swg @@ -703,6 +703,15 @@ public: template T SwigValueInit() { return T(); } +%} + +%insert("runtime") %{ +#if __cplusplus >=201103L +# define SWIG_STD_MOVE(OBJ) std::move(OBJ) +#else +# define SWIG_STD_MOVE(OBJ) OBJ +#endif + #endif %} #endif diff --git a/Lib/typemaps/swigtype.swg b/Lib/typemaps/swigtype.swg index 6973e3a10..52fac3252 100644 --- a/Lib/typemaps/swigtype.swg +++ b/Lib/typemaps/swigtype.swg @@ -417,7 +417,7 @@ /* directorin */ %typemap(directorin,noblock=1) SWIGTYPE { - $input = SWIG_NewPointerObj(%as_voidptr(new $1_ltype((const $1_ltype &)$1)), $&descriptor, SWIG_POINTER_OWN | %newpointer_flags); + $input = SWIG_NewPointerObj(%as_voidptr(new $1_ltype(SWIG_STD_MOVE($1))), $&descriptor, SWIG_POINTER_OWN | %newpointer_flags); } %typemap(directorin,noblock=1) SWIGTYPE * {