<div dir="ltr">Hi all,<br><br>Thanks for your reply. <div class="gmail_extra"><br>I'm very happy to contribute to Rcpp Gallery. </div><div class="gmail_extra"><br></div><div class="gmail_extra">I wrote some wrappers of C structure from 3rd party library with Rcpp, and these issues kept bothering me. A good example will make the life easier.</div>
<div class="gmail_extra"><br><div class="gmail_quote">2013/7/23 Dirk Eddelbuettel <span dir="ltr"><<a href="mailto:edd@debian.org" target="_blank">edd@debian.org</a>></span><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Hi Wush,<br>
<br>
Nice work on Rhiredis. I had some time to look more closely and have some<br>
comments below. In sum, you may have made things too complicated. But hey,<br>
you have a full working example which isn't too bad! Well done.<br>
<br>
On 22 July 2013 at 06:57, Dirk Eddelbuettel wrote:<br>
| On 22 July 2013 at 13:19, <a href="mailto:romain@r-enthusiasts.com">romain@r-enthusiasts.com</a> wrote:<br>
| | Le 2013-07-22 10:12, Wush Wu a écrit :<br>
<div class="im">| | > I wrote a wrapper of hiredis, which is a minimalistic C client for<br>
| | > the Redis database. Its name is `Rhiredis` and is much faster than<br>
| | > rredis, an existed redis client of R. Please<br>
</div>| | > see <a href="http://rpubs.com/wush978/rhiredis" target="_blank">http://rpubs.com/wush978/rhiredis</a> [1] for details. <br>
<br>
[ Actually, part the speed difference comes from you using base64enc which is<br>
C-based and faster than what rredis has. If you first create a base64 object,<br>
and then send that object directly with Rhiredis, the difference is smaller<br>
but of course still significant. ]<br></blockquote><div><br></div><div>Well, I didn't fully understand why rredis is so slow. As shown in my benchmark, the serialization of R object spends a little time(about 0.01 second), so I guess the main difference is the efficiency of transferring data between R and redis.</div>
<div><br></div><div>By the way, I talked to another redis user at Taiwan R User Group. He suggested me to make the API compatible with rredis so that Rhiredis could replace rredis in doRedis, which is one of parallel backend of `foreach`.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im"><br>
| | > However, I still want to know if there is a better approach. I tried<br>
| | > three approaches. Here is my opinions.<br>
| | ><br>
| | > # Preliminary<br>
| | ><br>
| | > Given a 3rd party C library, it usually provides a C structure and<br>
| | > functions of memory management. For example:<br>
| | ><br>
| | > ```c<br>
| | ><br>
| | > struct A {<br>
| | > int flag;<br>
| | > // ...<br>
| | > };<br>
| | ><br>
| | > A* initA();<br>
| | > void freeA(A* a);<br>
| | > ```<br>
| | ><br>
| | > The structure `A` need to be stored into a R object and pass to<br>
| | > another function.<br>
| | > Also, the user might need to access `A.flag`. For simplicity, there<br>
| | > is<br>
| | > only one field in this example. However, there might be a number of<br>
| | > fields in practice.<br>
<br>
</div>I think this is too complicated.<br>
<br>
All you need is 'static pointer' to the redic context object.<br>
<br>
Then create one function to instantiate the object, and reuse it.<br>
<div class="im"><br></div></blockquote><div><br></div><div>Sometimes I need to connect multiple redis server simultaneously, so I need to expose the connection object to make me select the server in R. </div><div><br></div>
<div>It is convenient to use a global object if the user only needs one connection. Therefore, I suggest to expose connection object to R, and let R put the last constructed object into `options` and pass it to the API as the default argument. This is how I implement Rhiredis.</div>
<div> </div><div><div>However, thanks for the idea of singleton which I have never thought before. Indeed, it is simpler.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">| | > However, it also produces memory leak because no `freeA` is called.<br>
| | ><br>
| | > Adding `.finalizer(freeA)` in `RCPP_MODULE` will cause an error of<br>
| | > freeing memory twice.<br>
| |<br>
</div>| | Gotta look into why this happens.<br>
<br></blockquote><div><br></div><div>I might misuse the `finalizer`. The `freeA` free the memory which will be freed by R later. However, I cannot find another place to call the `freeA` if I directly expose `A` with `RCPP_MODULE`.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I suspect this may be due to your use of Boost smart_ptr so you may be<br>
freeing something that might already be free'ed.<br>
<div class="im"><br></div></blockquote><div><br></div><div>The code in the repository is the third approach. Sorry that I didn't explain it clearly in the first mail.</div><div><br></div><div>The reason of using smart pointer is subtle.</div>
<div><br></div><div>At first, I embeded the connection structure into a C++ class like what Dirk showed below. I'll explain the problem there.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">| | > # Embed `A` into C++ class and expose the class with RCPP_MODULE<br>
| | ><br>
| | > This approach is implemented in `Rhiredis`.<br>
| | ><br>
| | > Finally, I still need to write helper function to expose the field of<br>
| | > `A`. But the user could access these flag in R with operator `$`.<br>
| | ><br>
| | > Note that I still need a function to extract the pointer of `A` from<br>
| | > exposed S4 object:<br>
| | ><br>
| | > ```cpp<br>
| | ><br>
| | > template<class T><br>
| | > T* extract_ptr(SEXP s) {<br>
| | > Rcpp::S4 s4(s);<br>
| | > Rcpp::Environment env(s4);<br>
| | > Rcpp::XPtr<T> xptr(env.get(".pointer"));<br>
| | > return static_cast<T*>(R_ExternalPtrAddr(xptr));<br>
| | > }<br>
| | > ```<br>
<br>
</div>I do not understand why you would need all that.<br>
<div class="im"><br></div></blockquote><div><br></div><div>Take `redisCommand` for example. Since I didn't make the connection object to be a singleton, R need to pass a connection object to `redisCommand`. The object is exposed by `RCPP_MODULE`, so I need to extract it from `RCPP_MODULE`.</div>
<div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">
| | > Please give me some suggestion if you know a better or a different<br>
| | > approach.<br>
| |<br>
</div>| | I would essentially do what you do: use RCPP_MODULE to expose a C++<br>
| | class so that the C++ class manages scoping : constructor, destructor,<br>
| | etc ...<br>
| |<br>
| | class A_cpp {<br>
| | public:<br>
| | A_cpp( ) : obj( initA() ){}<br>
| | ~A_cpp(){ freeA(obj); obj = NULL ; }<br>
| |<br>
| | int get_flag(){ return obj->flag ; }<br>
| | void set_flag( int x ){ obj->flag = x ; }<br>
| |<br>
| | private:<br>
| | A* obj ;<br>
| | } ;<br>
<br></blockquote><div><br></div><div><div>In my case, a double free runtime error occurred. The reason is due to the copy constructor of class A_cpp, i.e. `A_cpp(const A_cpp& src);`. </div><div><br></div><div>This implementation will corrupt memory if:<br>
</div><div><br></div><div>```cpp</div><div> A_cpp a1;<br></div><div> A_cpp a2(a1);</div><div>```<br></div><div> </div><div>The compiler will free the space of a1::obj twice. The first time is at `a1::~A_cpp` and the second time is at `a2::~A_cpp`.</div>
<div><br></div><div>That's why I use a smart pointer to make class A_cpp copyable.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
</blockquote></div><div>The implementation of RCPP_MODULE contains a copy construction. A compile-time error occurred if I close the copy constructor manually:</div><div><br></div><div>```cpp</div><div>class A_cpp {</div>
<div> //...</div><div>private:</div><div> A_cpp(const A_cpp&);</div><div> void operator=(const A_cpp&);</div><div>}</div><div>```</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
The code is for now in a quick fork of your Rhiredis in my github account.<br>
<br>
Hope this helps, Dirk<br>
<span class=""> <font color="#888888"><br></font></span></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><font color="#888888">
--<br>
Dirk Eddelbuettel | <a href="mailto:edd@debian.org">edd@debian.org</a> | <a href="http://dirk.eddelbuettel.com" target="_blank">http://dirk.eddelbuettel.com</a><br>
</font></span></blockquote></div><br></div><div class="gmail_extra">Thank you, Dirk.</div><div class="gmail_extra"><br></div><div class="gmail_extra">A singleton does make things easier. However, I still want to know how to expose C structure with provided `free`-like function because sometimes we cannot use singleton.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Wush</div></div>