Re: [Rd] suggestion how to use memcpy in duplicate.c

From: Simon Urbanek <simon.urbanek_at_r-project.org>
Date: Wed, 21 Apr 2010 16:48:08 -0400

On Apr 21, 2010, at 4:39 PM, Simon Urbanek wrote:

> 
> On Apr 21, 2010, at 4:13 PM, Romain Francois wrote:
> 
>> Le 21/04/10 21:39, Simon Urbanek a écrit :
>>> 
>>> 
>>> On Apr 21, 2010, at 3:32 PM, Romain Francois wrote:
>>> 
>>>> Le 21/04/10 17:54, Matthew Dowle a écrit :

>>>>>
>>>>>> From copyVector in duplicate.c :

>>>>>
>>>>> void copyVector(SEXP s, SEXP t)
>>>>> {
>>>>> int i, ns, nt;
>>>>> nt = LENGTH(t);
>>>>> ns = LENGTH(s);
>>>>> switch (TYPEOF(s)) {
>>>>> ...
>>>>> case INTSXP:
>>>>> for (i = 0; i< ns; i++)
>>>>> INTEGER(s)[i] = INTEGER(t)[i % nt];
>>>>> break;
>>>>> ...
>>>>>
>>>>> could that be replaced with :
>>>>>
>>>>> case INTSXP:
>>>>> for (i=0; i<ns/nt; i++)
>>>>> memcpy((char *)DATAPTR(s)+i*nt*sizeof(int), (char *)DATAPTR(t),
>>>>> nt*sizeof(int));
>>>>> break;
>>>> 
>>>> or at least with something like this:
>>>> 
>>>> int* p_s = INTEGER(s) ;
>>>> int* p_t = INTEGER(t) ;
>>>> for( i=0 ; i<  ns ; i++){
>>>> 	p_s[i] = p_t[i % nt];
>>>> }
>>>> 
>>>> since expanding the INTEGER macro over and over has a price.
>>>> 
>>> 
>>> ... in packages, yes, but not in core R.
>> 
>> Hmm. I was not talking about the overhead of the INTEGER function:
>> 
>> int *(INTEGER)(SEXP x) {
>>   if(TYPEOF(x) != INTSXP && TYPEOF(x) != LGLSXP)
>> 	error("%s() can only be applied to a '%s', not a '%s'",
>> 	      "INTEGER", "integer", type2char(TYPEOF(x)));
>>   return INTEGER(x);
>> }
>> 
>> 
>> 
>> but the one related to the macro.
>> 
>> #define INTEGER(x)	((int *) DATAPTR(x))
>> #define DATAPTR(x)	(((SEXPREC_ALIGN *) (x)) + 1)
>> 
>> so the loop expands to :
>> 
>> for (i = 0; i<   ns; i++)
>>         ((int *) (((SEXPREC_ALIGN *) (s)) + 1))[i] = ((int *) (((SEXPREC_ALIGN *) (t)) + 1))[i % nt];
>> 
>> I still believe grabbing the pointer just once for s and once for t is more efficient ...
>> 
> 
> Nope, since everything involved is loop invariant so the pointer values don't change (you'd have to declare s or t volatile to prevent that).
> 
> Try using gcc -s

Sorry, I meant gcc -S of course.

> and you'll see that the code is the same (depending on the version of gcc the order of the first comparison can change so technically the INTEGER(x) version can save one add instruction in the degenerate case and be faster(!) in old gcc

[the reason being that INTEGER(x) does not need to be evaluated if the loop is not entered whereas p_s and p_t are populated unconditionally - smarter compilers will notice that p_s/p_t are not used in that case and thus generate identical code in both cases]

Cheers,
Simon

> 
> 
>>> 

>>>>> and similar for the other types in copyVector. This won't help regular
>>>>> vector copies, since those seem to be done by the DUPLICATE_ATOMIC_VECTOR
>>>>> macro, see next suggestion below, but it should help copyMatrix which calls
>>>>> copyVector, scan.c which calls copyVector on three lines, dcf.c (once) and
>>>>> dounzip.c (once).
>>>>>
>>>>> For the DUPLICATE_ATOMIC_VECTOR macro there is already a comment next to it
>>>>> :
>>>>>
>>>>> <FIXME>: surely memcpy would be faster here?
>>>>>
>>>>> which seems to refer to the for loop :
>>>>>
>>>>> else { \
>>>>> int __i__; \
>>>>> type *__fp__ = fun(from), *__tp__ = fun(to); \
>>>>> for (__i__ = 0; __i__< __n__; __i__++) \
>>>>> __tp__[__i__] = __fp__[__i__]; \
>>>>> } \
>>>>>
>>>>> Could that loop be replaced by the following ?
>>>>>
>>>>> else { \
>>>>> memcpy((char *)DATAPTR(to), (char *)DATAPTR(from), __n__*sizeof(type)); \
>>>>> }\
>>>>>
>>>>> In the data.table package, dogroups.c uses this technique, so the principle
>>>>> is tested and works well so far.
>>>>>
>>>>> Are there any road blocks preventing this change, or is anyone already
>>>>> working on it ? If not then I'll try and test it (on Ubuntu 32bit) and
>>>>> submit patch with timings, as before. Comments/pointers much appreciated.
>>>>>
>>>>> Matthew
>> 
>> 
>> -- 
>> Romain Francois
>> Professional R Enthusiast
>> +33(0) 6 28 91 30 30
>> http://romainfrancois.blog.free.fr
>> |- http://bit.ly/9aKDM9 : embed images in Rd documents
>> |- http://tr.im/OIXN : raster images and RImageJ
>> |- http://tr.im/OcQe : Rcpp 0.7.7
>> 
>> 
>> 
> 
> ______________________________________________
> R-devel_at_r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
> 
> 

______________________________________________
R-devel_at_r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel Received on Wed 21 Apr 2010 - 20:52:54 GMT

Archive maintained by Robert King, hosted by the discipline of statistics at the University of Newcastle, Australia.
Archive generated by hypermail 2.2.0, at Thu 22 Apr 2010 - 00:30:17 GMT.

Mailing list information is available at https://stat.ethz.ch/mailman/listinfo/r-devel. Please read the posting guide before posting to the list.

list of date sections of archive