Skip to content

remove some DATAPTR #3350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattdowle merged 2 commits into from
Feb 4, 2019
Merged

remove some DATAPTR #3350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3738443
remove DATAPTR, #3301
jangorecki Feb 4, 2019
2bf3183
const SEXP *STRING_PTR
mattdowle Feb 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions src/fmelt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,23 +441,23 @@ SEXP getvaluecols(SEXP DT, SEXP dtnames, Rboolean valfactor, Rboolean verbose, s
for (k=0; k<thislen; k++)
REAL(target)[counter + k] = REAL(thiscol)[INTEGER(thisidx)[k]-1];
} else {
memcpy((char *)DATAPTR(target)+j*data->nrow*size, (char *)DATAPTR(thiscol), data->nrow*size);
memcpy((char *)REAL(target)+j*data->nrow*size, (char *)REAL(thiscol), data->nrow*size);
}
break;
case INTSXP :
if (data->narm) {
for (k=0; k<thislen; k++)
INTEGER(target)[counter + k] = INTEGER(thiscol)[INTEGER(thisidx)[k]-1];
} else {
memcpy((char *)DATAPTR(target)+j*data->nrow*size, (char *)DATAPTR(thiscol), data->nrow*size);
memcpy((char *)INTEGER(target)+j*data->nrow*size, (char *)INTEGER(thiscol), data->nrow*size);
}
break;
case LGLSXP :
if (data->narm) {
for (k=0; k<thislen; k++)
LOGICAL(target)[counter + k] = LOGICAL(thiscol)[INTEGER(thisidx)[k]-1];
} else {
memcpy((char *)DATAPTR(target)+j*data->nrow*size, (char *)DATAPTR(thiscol), data->nrow*size);
memcpy((char *)LOGICAL(target)+j*data->nrow*size, (char *)LOGICAL(thiscol), data->nrow*size);
}
break;
default : error("Unknown column type '%s' for column '%s'.", type2char(TYPEOF(thiscol)), CHAR(STRING_ELT(dtnames, INTEGER(thisvaluecols)[i]-1)));
Expand Down Expand Up @@ -573,7 +573,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
}
} else {
for (j=0; j<data->lmax; j++)
memcpy((char *)DATAPTR(target)+j*data->nrow*size, (char *)DATAPTR(thiscol), data->nrow*size);
memcpy((char *)REAL(target)+j*data->nrow*size, (char *)REAL(thiscol), data->nrow*size);
}
break;
case INTSXP :
Expand All @@ -587,7 +587,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
}
} else {
for (j=0; j<data->lmax; j++)
memcpy((char *)DATAPTR(target)+j*data->nrow*size, (char *)DATAPTR(thiscol), data->nrow*size);
memcpy((char *)INTEGER(target)+j*data->nrow*size, (char *)INTEGER(thiscol), data->nrow*size);
}
break;
case LGLSXP :
Expand All @@ -601,7 +601,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
}
} else {
for (j=0; j<data->lmax; j++)
memcpy((char *)DATAPTR(target)+j*data->nrow*size, (char *)DATAPTR(thiscol), data->nrow*size);
memcpy((char *)LOGICAL(target)+j*data->nrow*size, (char *)LOGICAL(thiscol), data->nrow*size);
}
break;
case STRSXP :
Expand All @@ -614,11 +614,12 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
counter += thislen;
}
} else {
// SET_STRING_ELT for j=0 and memcpy for j>0, WHY?
// From assign.c's memcrecycle - only one SET_STRING_ELT per RHS item is needed to set generations (overhead)
for (k=0; k<data->nrow; k++) SET_STRING_ELT(target, k, STRING_ELT(thiscol, k));
for (j=1; j<data->lmax; j++)
memcpy((char *)DATAPTR(target)+j*data->nrow*size, (char *)DATAPTR(target), data->nrow*size);
const SEXP *s = STRING_PTR(thiscol); // to reduce overhead of STRING_ELT() inside loop below. Read-only hence const.
for (j=0; j<data->lmax; j++) {
for (k=0; k<data->nrow; k++) {
SET_STRING_ELT(target, j*data->nrow + k, s[k]);
}
}
}
break;
case VECSXP :
Expand Down
4 changes: 2 additions & 2 deletions src/gsumm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ SEXP gmedian(SEXP x, SEXP narm) {
isint64 = (isString(klass) && STRING_ELT(klass, 0) == char_integer64);
ans = PROTECT(allocVector(REALSXP, ngrp));
sub = PROTECT(allocVector(REALSXP, maxgrpn)); // allocate once upfront
ptr = DATAPTR(sub);
ptr = REAL(sub);
if (!LOGICAL(narm)[0]) {
for (i=0; i<ngrp; i++) {
isna = FALSE;
Expand Down Expand Up @@ -876,7 +876,7 @@ SEXP gmedian(SEXP x, SEXP narm) {
case LGLSXP: case INTSXP:
ans = PROTECT(allocVector(REALSXP, ngrp));
sub = PROTECT(allocVector(INTSXP, maxgrpn)); // allocate once upfront
ptr = DATAPTR(sub);
ptr = INTEGER(sub);
if (!LOGICAL(narm)[0]) {
for (i=0; i<ngrp; i++) {
isna = FALSE;
Expand Down
15 changes: 9 additions & 6 deletions src/rbindlist.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,10 @@ SEXP rbindlist(SEXP l, SEXP sexp_usenames, SEXP sexp_fill, SEXP idcol) {
// TO DO: options(datatable.pedantic=TRUE) to issue this warning :
// warning("Column %d of item %d is type '%s', inconsistent with column %d of item %d's type ('%s')",j+1,i+1,type2char(TYPEOF(thiscol)),j+1,first+1,type2char(TYPEOF(target)));
}
if (TYPEOF(target)!=STRSXP && TYPEOF(thiscol)!=TYPEOF(target)) {
error("Internal error in rbindlist.c: type of 'thiscol' [%s] should have already been coerced to 'target' [%s]. please report to data.table issue tracker.",
type2char(TYPEOF(thiscol)), type2char(TYPEOF(target)));
}
switch(TYPEOF(target)) {
case STRSXP :
isRowOrdered[resi] = FALSE;
Expand Down Expand Up @@ -723,22 +727,21 @@ SEXP rbindlist(SEXP l, SEXP sexp_usenames, SEXP sexp_fill, SEXP idcol) {
}
break;
case VECSXP :
if (TYPEOF(thiscol) != VECSXP) error("Internal logical error in rbindlist.c (not VECSXP), please report to data.table issue tracker.");
for (r=0; r<thislen; r++)
SET_VECTOR_ELT(target, ansloc+r, VECTOR_ELT(thiscol,r));
break;
case CPLXSXP : // #1659 fix
if (TYPEOF(thiscol) != TYPEOF(target)) error("Internal logical error in rbindlist.c, type of 'thiscol' should have already been coerced to 'target'. please report to data.table issue tracker.");
for (r=0; r<thislen; r++)
COMPLEX(target)[ansloc+r] = COMPLEX(thiscol)[r];
break;
case REALSXP:
memcpy(REAL(target)+ansloc, REAL(thiscol), thislen*SIZEOF(thiscol));
break;
case INTSXP:
memcpy(INTEGER(target)+ansloc, INTEGER(thiscol), thislen*SIZEOF(thiscol));
break;
case LGLSXP:
if (TYPEOF(thiscol) != TYPEOF(target)) error("Internal logical error in rbindlist.c, type of 'thiscol' should have already been coerced to 'target'. please report to data.table issue tracker.");
memcpy((char *)DATAPTR(target) + ansloc * SIZEOF(thiscol),
(char *)DATAPTR(thiscol),
thislen * SIZEOF(thiscol));
memcpy(LOGICAL(target)+ansloc, LOGICAL(thiscol), thislen*SIZEOF(thiscol));
break;
default :
error("Unsupported column type '%s'", type2char(TYPEOF(target)));
Expand Down
9 changes: 6 additions & 3 deletions src/uniqlist.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ SEXP uniqlist(SEXP l, SEXP order)
int *o = INTEGER(order); // only used when via_order is true
switch(TYPEOF(v)) {
case INTSXP : case LGLSXP : {
int *vd=INTEGER(v), prev, elem;
const int *vd=INTEGER(v);
int prev, elem;
if (via_order) {
// ad hoc by (order passed in)
COMPARE1_VIA_ORDER COMPARE2
Expand All @@ -65,15 +66,17 @@ SEXP uniqlist(SEXP l, SEXP order)
}
} break;
case STRSXP : {
SEXP *vd=(SEXP *)DATAPTR(v), prev, elem; // TODO: tried to replace DATAPTR here but (SEXP *)&STRING_ELT(v,0) results in lvalue required as unary ‘&’ operand
const SEXP *vd=STRING_PTR(v);
SEXP prev, elem;
if (via_order) {
COMPARE1_VIA_ORDER && ENC2UTF8(elem)!=ENC2UTF8(prev) COMPARE2 // but most of the time they are equal, so ENC2UTF8 doesn't need to be called
} else {
COMPARE1 && ENC2UTF8(elem)!=ENC2UTF8(prev) COMPARE2
}
} break;
case REALSXP : {
uint64_t *vd=(uint64_t *)REAL(v), prev, elem;
const uint64_t *vd=(const uint64_t *)REAL(v);
uint64_t prev, elem;
// grouping by integer64 makes sense (ids). grouping by float supported but a good use-case for that is harder to imagine
if (getNumericRounding_C()==0 /*default*/ || inherits(v, "integer64")) {
if (via_order) {
Expand Down