armgong / rjulia

R package which integrating R and Julia
GNU General Public License v2.0
146 stars 23 forks source link

unexpected slowness in r2j #29

Closed phaverty closed 8 years ago

phaverty commented 8 years ago

After some profiling, it looks like all of the time for r2j is going into RDims_JuliaTuple. I see some commented out code that looks like about the right solution. I tried the vector version and it seemed to do the right thing (very quickly). Can I help out with this in some way?

armgong commented 8 years ago

thanks a lot , i will trace it and see what's happens ,but kind of busy now,.

armgong commented 8 years ago

the comment code in RDims_JuliaTuple

/*jl_value_t*d=NULL;
 JL_GC_PUSH1(&d);
 SEXP dims = getAttrib(Var, R_DimSymbol);
  //array or matrix
  if (dims != R_NilValue)
  {
    int ndims = LENGTH(dims);
    d = jl_alloc_svec(ndims);
    for (size_t i = 0; i < ndims; i++)
    {
      jl_svecset(d, i, jl_box_long(INTEGER(dims)[i]));
    }
  }
  else     //vector
  {
    d = jl_alloc_svec(1);
    jl_svecset(d, 0, jl_box_long(LENGTH(Var)));
  }
  JL_GC_POP();
  return d;
 */

caused by julia 0.4 remove tuple api , add simplevector api , but svec is not tuple, so I change code to the new version. after your issue , did some digging , maybe jl_apply_tuple_type could be used in the old code, change svec to tuple, so we can use the old code.

 jl_value_t* newd= (jl_value_t*)jl_apply_tuple_type(d);
JL_GC_POP();
return newd;

Could you try this new code, and create new patch, thanks a lot

phaverty commented 8 years ago

Great, I'll give it a try, probably later in the week. julia also exports jl_alloc_array_1d, jl_alloc_array_2d, and jl_alloc_array_3d. We wouldn't need to figure out the tuple part for these most common cases.

Pete


Peter M. Haverty, Ph.D. Genentech, Inc. phaverty@gene.com

On Mon, Jun 20, 2016 at 6:26 PM, Yu Gong notifications@github.com wrote:

the comment code in RDims_JuliaTuple

/_jl_value_t_d=NULL; JL_GC_PUSH1(&d); SEXP dims = getAttrib(Var, R_DimSymbol); //array or matrix if (dims != R_NilValue) { int ndims = LENGTH(dims); d = jl_alloc_svec(ndims); for (size_t i = 0; i < ndims; i++) { jl_svecset(d, i, jl_box_long(INTEGER(dims)[i])); } } else //vector { d = jl_alloc_svec(1); jl_svecset(d, 0, jl_box_long(LENGTH(Var))); } JL_GC_POP(); return d; */

caused by julia 0.4 remove tuple api , add simplevector api , but svec is not tuple, so I change code to the new version. after your issue , did some digging , maybe jl_apply_tuple_type could be used in the old code, change svec to tuple, so we can use the old code.

jl_value_t* newd= (jl_value_t*)jl_apply_tuple_type(d);JL_GC_POP();return newd;

Could new try this new code, and create new patch, thanks a lot

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/armgong/rjulia/issues/29#issuecomment-227316434, or mute the thread https://github.com/notifications/unsubscribe/AH02K27iSQdp1y3wcKgi4FhNaMcLDmvgks5qNz3RgaJpZM4I58Rs .