From simon.urbanek at r-project.org  Wed Apr 21 21:39:58 2010
From: simon.urbanek at r-project.org (Simon Urbanek)
Date: Wed, 21 Apr 2010 15:39:58 -0400
Subject: [Rd] suggestion how to use memcpy in duplicate.c
In-Reply-To: <4BCF52C6.8020303@r-enthusiasts.com>
References: <hqn72q$uev$1@dough.gmane.org> <4BCF52C6.8020303@r-enthusiasts.com>
Message-ID: <895C584E-A9E5-4E56-B300-8E351080DFEC@r-project.org>


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.

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
>> 
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> 
>> 
> 
> 
> -- 
> 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
> 
> 



[Rd] suggestion how to use memcpy in duplicate.c
Simon Urbanek