Skip to content

Commit 766efde

Browse files
badasahogjangorecki
badasahog
and
jangorecki
authored
upgraded MEMCPY from macro to function (#7304)
* upgraded MEMCPY from macro to function Also: formatting improvements inlining of temporary objects constness improvements (both qualifiers and API functions) * make static inline, add codecov * codecov does not work well from parallel, use 1 th --------- Co-authored-by: Jan Gorecki <[email protected]>
1 parent ed2f36f commit 766efde

File tree

2 files changed

+50
-54
lines changed

2 files changed

+50
-54
lines changed

‎inst/tests/froll.Rraw‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,6 +1429,12 @@ test(6010.610, frollapply(numeric(), 3, identity), list())
14291429
test(6010.611, frollapply(list(numeric(), numeric()), 3, identity), list(NULL,NULL))
14301430
test(6010.612, frollapply(list(numeric(), 1:3), 3, identity), list(NULL, list(NA,NA,1:3)))
14311431

1432+
## codecov memcpy calls #7304
1433+
x = data.table(v1=1:2, v2=c(1,2), v3=c("a","b"), v4=list(1,2), v5=as.complex(1:2), v6=as.raw(1:2))
1434+
old = setDTthreads(1L) ## ensure no parallel::mccollect so codecov can pick up
1435+
test(6010.619, frollapply(x, 1, ncol, by.column=FALSE), c(6L,6L))
1436+
setDTthreads(old)
1437+
14321438
#### list input in frollapply
14331439
DT = as.data.table(iris)
14341440
test(6010.620, ## list()/.() same as data.frame()

‎src/frollapply.c‎

Lines changed: 44 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,74 @@
11
#include "data.table.h"
22

3-
#define MEMCPY \
4-
switch (TYPEOF(d)) { \
5-
case INTSXP: { \
6-
memcpy(INTEGER(d), INTEGER(s)+o, nrow*sizeof(int)); \
7-
} break; \
8-
case LGLSXP: { \
9-
memcpy(LOGICAL(d), LOGICAL(s)+o, nrow*sizeof(int)); \
10-
} break; \
11-
case REALSXP: { \
12-
memcpy(REAL(d), REAL(s)+o, nrow*sizeof(double)); \
13-
} break; \
14-
case STRSXP: { \
15-
for (int i=0; i<nrow; ++i) \
16-
SET_STRING_ELT(d, i, STRING_ELT(s, o + i)); \
17-
} break; \
18-
case VECSXP: { \
19-
for (int i=0; i<nrow; ++i) \
20-
SET_VECTOR_ELT(d, i, VECTOR_ELT(s, o + i)); \
21-
} break; \
22-
case CPLXSXP: { \
23-
memcpy(COMPLEX(d), COMPLEX(s)+o, nrow*sizeof(Rcomplex)); \
24-
} break; \
25-
case RAWSXP : { \
26-
memcpy(RAW(d), RAW(s)+o, nrow*sizeof(Rbyte)); \
27-
} break; \
28-
default: internal_error(__func__, "column type not supported in memcpyVector or memcpyDT function. All known types are supported so please report as bug."); \
29-
} \
3+
static inline void MEMCPY_SEXP(SEXP dest, size_t offset, SEXP src, int count)
4+
{
5+
switch (TYPEOF(dest)) {
6+
case INTSXP: {
7+
memcpy(INTEGER(dest), INTEGER_RO(src) + offset, count * sizeof(int));
8+
} break;
9+
case REALSXP: {
10+
memcpy(REAL(dest), REAL_RO(src) + offset, count * sizeof(double));
11+
} break;
12+
case STRSXP: {
13+
for (int i = 0; i < count; i++)
14+
SET_STRING_ELT(dest, i, STRING_ELT(src, offset + i));
15+
} break;
16+
case VECSXP: {
17+
for (int i = 0; i < count; i++)
18+
SET_VECTOR_ELT(dest, i, VECTOR_ELT(src, offset + i));
19+
} break;
20+
case CPLXSXP: {
21+
memcpy(COMPLEX(dest), COMPLEX_RO(src) + offset, count * sizeof(Rcomplex));
22+
} break;
23+
case RAWSXP: {
24+
memcpy(RAW(dest), RAW_RO(src) + offset, count * sizeof(Rbyte));
25+
} break;
26+
default: internal_error(__func__, "column type not supported in memcpyVector or memcpyDT function. All known types are supported so please report as bug."); // # nocov
27+
}
28+
}
3029

3130
/*
3231
* memcpy src data into preallocated window
3332
* we don't call memcpyVector from memcpyDT because they are called in tight loop and we don't want to have extra branches inside
3433
*/
3534
SEXP memcpyVector(SEXP dest, SEXP src, SEXP offset, SEXP size) {
36-
size_t o = INTEGER(offset)[0] - INTEGER(size)[0];
37-
int nrow = LENGTH(dest);
38-
SEXP d = dest, s = src;
39-
MEMCPY
35+
MEMCPY_SEXP(dest, INTEGER_RO(offset)[0] - INTEGER_RO(size)[0], src, LENGTH(dest));
4036
return dest;
4137
}
4238
// # nocov start ## does not seem to be reported to codecov most likely due to running in a fork, I manually debugged that it is being called when running froll.Rraw
4339
SEXP memcpyDT(SEXP dest, SEXP src, SEXP offset, SEXP size) {
4440
//Rprintf("%d",1); // manual code coverage to confirm it is reached when marking nocov
45-
size_t o = INTEGER(offset)[0] - INTEGER(size)[0];
46-
int ncol = LENGTH(dest), nrow = LENGTH(VECTOR_ELT(dest, 0));
47-
SEXP d, s;
48-
for (int j=0; j<ncol; ++j) {
49-
d = VECTOR_ELT(dest, j);
50-
s = VECTOR_ELT(src, j);
51-
MEMCPY
41+
const int ncol = LENGTH(dest);
42+
const int nrow = LENGTH(VECTOR_ELT(dest, 0));
43+
for (int i = 0; i < ncol; i++) {
44+
MEMCPY_SEXP(VECTOR_ELT(dest, i), INTEGER_RO(offset)[0] - INTEGER_RO(size)[0], VECTOR_ELT(src, i), nrow);
5245
}
5346
return dest;
5447
}
5548
// # nocov end
5649

5750
SEXP memcpyVectoradaptive(SEXP dest, SEXP src, SEXP offset, SEXP size) {
58-
size_t oi = INTEGER(offset)[0];
59-
int nrow = INTEGER(size)[oi-1];
60-
size_t o = oi - nrow; // oi should always be bigger than nrow because we filter out incomplete window using ansMask
61-
SEXP d = dest, s = src;
62-
SETLENGTH(d, nrow); // must be before MEMCPY because attempt to set index 1/1 in SET_STRING_ELT test 6010.150
51+
const size_t oi = INTEGER_RO(offset)[0];
52+
const int nrow = INTEGER_RO(size)[oi - 1];
53+
const size_t o = oi - nrow; // oi should always be bigger than nrow because we filter out incomplete window using ansMask
54+
SETLENGTH(dest, nrow); // must be before MEMCPY_SEXP because attempt to set index 1/1 in SET_STRING_ELT test 6010.150
6355
if (nrow) { // support k[i]==0
64-
MEMCPY
56+
MEMCPY_SEXP(dest, o, src, nrow);
6557
}
6658
return dest;
6759
}
6860
// # nocov start ## does not seem to be reported to codecov most likely due to running in a fork, I manually debugged that it is being called when running froll.Rraw
6961
SEXP memcpyDTadaptive(SEXP dest, SEXP src, SEXP offset, SEXP size) {
7062
//Rprintf("%d",2); // manual code coverage to confirm it is reached when marking nocov
71-
size_t oi = INTEGER(offset)[0];
72-
int nrow = INTEGER(size)[oi-1];
73-
size_t o = oi - nrow;
74-
int ncol = LENGTH(dest);
75-
SEXP d, s;
76-
for (int j=0; j<ncol; ++j) {
77-
d = VECTOR_ELT(dest, j);
78-
s = VECTOR_ELT(src, j);
63+
size_t oi = INTEGER_RO(offset)[0];
64+
const int nrow = INTEGER_RO(size)[oi - 1];
65+
const size_t o = oi - nrow;
66+
const int ncol = LENGTH(dest);
67+
for (int i = 0; i < ncol; i++) {
68+
SEXP d = VECTOR_ELT(dest, i);
7969
SETLENGTH(d, nrow);
8070
if (nrow) { // support k[i]==0
81-
MEMCPY
71+
MEMCPY_SEXP(d, o, VECTOR_ELT(src, i), nrow);
8272
}
8373
}
8474
return dest;
@@ -92,7 +82,7 @@ SEXP setgrowable(SEXP x) {
9282
SET_TRUELENGTH(x, LENGTH(x)); // important because gc() uses TRUELENGTH to keep counts
9383
} else {
9484
// # nocov start ## does not seem to be reported to codecov most likely due to running in a fork, I manually debugged that it is being called when running froll.Rraw
95-
for (int i=0; i<LENGTH(x); ++i) {
85+
for (int i = 0; i < LENGTH(x); i++) {
9686
//Rprintf("%d",3); // manual code coverage to confirm it is reached when marking nocov
9787
SEXP col = VECTOR_ELT(x, i);
9888
SET_GROWABLE_BIT(col);

0 commit comments

Comments
 (0)