From 7e425e78050dc20cab6db7adc344b18db6f9ae4d Mon Sep 17 00:00:00 2001 From: mattdowle Date: Wed, 23 May 2018 17:08:55 -0700 Subject: [PATCH 1/4] between.c --- src/between.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/between.c b/src/between.c index 678016fd2e..19d848c327 100644 --- a/src/between.c +++ b/src/between.c @@ -3,51 +3,51 @@ static double l=0.0, u=0.0; -Rboolean int_upper_closed(SEXP x, R_len_t i) { +static Rboolean int_upper_closed(SEXP x, R_len_t i) { return (INTEGER(x)[i] == NA_INTEGER || (double)INTEGER(x)[i] <= u ? NA_LOGICAL : FALSE); } -Rboolean int_upper_open(SEXP x, R_len_t i) { +static Rboolean int_upper_open(SEXP x, R_len_t i) { return (INTEGER(x)[i] == NA_INTEGER || (double)INTEGER(x)[i] < u ? NA_LOGICAL : FALSE); } -Rboolean int_lower_closed(SEXP x, R_len_t i) { +static Rboolean int_lower_closed(SEXP x, R_len_t i) { return (INTEGER(x)[i] == NA_INTEGER || (double)INTEGER(x)[i] >= l ? NA_LOGICAL : FALSE); } -Rboolean int_lower_open(SEXP x, R_len_t i) { +static Rboolean int_lower_open(SEXP x, R_len_t i) { return (INTEGER(x)[i] == NA_INTEGER || (double)INTEGER(x)[i] > l ? NA_LOGICAL : FALSE); } -Rboolean int_both_closed(SEXP x, R_len_t i) { +static Rboolean int_both_closed(SEXP x, R_len_t i) { return (INTEGER(x)[i] == NA_INTEGER ? NA_LOGICAL : ((double)INTEGER(x)[i] >= l && (double)INTEGER(x)[i] <= u)); } -Rboolean int_both_open(SEXP x, R_len_t i) { +static Rboolean int_both_open(SEXP x, R_len_t i) { return (INTEGER(x)[i] == NA_INTEGER ? NA_LOGICAL : ((double)INTEGER(x)[i] > l && (double)INTEGER(x)[i] < u)); } -Rboolean double_upper_closed(SEXP x, R_len_t i) { +static Rboolean double_upper_closed(SEXP x, R_len_t i) { return (ISNAN(REAL(x)[i]) || REAL(x)[i] <= u ? NA_LOGICAL : FALSE); } -Rboolean double_upper_open(SEXP x, R_len_t i) { +static Rboolean double_upper_open(SEXP x, R_len_t i) { return (ISNAN(REAL(x)[i]) || REAL(x)[i] < u ? NA_LOGICAL : FALSE); } -Rboolean double_lower_closed(SEXP x, R_len_t i) { +static Rboolean double_lower_closed(SEXP x, R_len_t i) { return (ISNAN(REAL(x)[i]) || REAL(x)[i] >= l ? NA_LOGICAL : FALSE); } -Rboolean double_lower_open(SEXP x, R_len_t i) { +static Rboolean double_lower_open(SEXP x, R_len_t i) { return (ISNAN(REAL(x)[i]) || REAL(x)[i] > l ? NA_LOGICAL : FALSE); } -Rboolean double_both_closed(SEXP x, R_len_t i) { +static Rboolean double_both_closed(SEXP x, R_len_t i) { return (ISNAN(REAL(x)[i]) ? NA_LOGICAL : (REAL(x)[i] >= l && REAL(x)[i] <= u)); } -Rboolean double_both_open(SEXP x, R_len_t i) { +static Rboolean double_both_open(SEXP x, R_len_t i) { return (ISNAN(REAL(x)[i]) ? NA_LOGICAL : (REAL(x)[i] > l && REAL(x)[i] < u)); } @@ -66,11 +66,18 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP bounds) { if (!isLogical(bounds) || LOGICAL(bounds)[0] == NA_LOGICAL) error("incbounds must be logical TRUE/FALSE."); + int nprotect = 0; + if (ALTREP(x)) { x = PROTECT(duplicate(x)); nprotect++; } + if (ALTREP(lower)) { lower = PROTECT(duplicate(lower)); nprotect++; } + if (ALTREP(upper)) { upper = PROTECT(duplicate(upper)); nprotect++; } + if (ALTREP(bounds)) { bounds = PROTECT(duplicate(bounds)); nprotect++; } + // no support for int64 yet (only handling most common cases) // coerce to also get NA values properly lower = PROTECT(coerceVector(lower, REALSXP)); l = REAL(lower)[0]; upper = PROTECT(coerceVector(upper, REALSXP)); u = REAL(upper)[0]; ans = PROTECT(allocVector(LGLSXP, nx)); + nprotect += 3; if (LOGICAL(bounds)[0]) { fupper = isInteger(x) ? &int_upper_closed : &double_upper_closed; @@ -99,6 +106,6 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP bounds) { for (i=0; i Date: Wed, 23 May 2018 21:42:38 -0700 Subject: [PATCH 2/4] fsort.c --- src/fsort.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fsort.c b/src/fsort.c index 7a639c76bb..f63676b99c 100644 --- a/src/fsort.c +++ b/src/fsort.c @@ -112,6 +112,7 @@ SEXP fsort(SEXP x, SEXP verboseArg) { // TODO: not only detect if already sorted, but if it is, just return x to save the duplicate SEXP ansVec = PROTECT(allocVector(REALSXP, xlength(x))); + int nprotect = 1; double *ans = REAL(ansVec); // allocate early in case fails if not enough RAM // TODO: document this is much cheaper than a copy followed by in-place. @@ -127,6 +128,8 @@ SEXP fsort(SEXP x, SEXP verboseArg) { // could be that lastBatchSize == batchSize when i) xlength(x) is multiple of nBatch // and ii) for small vectors with just one batch + if (ALTREP(x)) { x = PROTECT(duplicate(x)); nprotect++; } + t[1] = wallclock(); double mins[nBatch], maxs[nBatch]; #pragma omp parallel for schedule(dynamic) num_threads(nth) @@ -305,8 +308,7 @@ SEXP fsort(SEXP x, SEXP verboseArg) { Rprintf("%d: %.3f (%4.1f%%)\n", i, t[i]-t[i-1], 100.*(t[i]-t[i-1])/tot); } - UNPROTECT(1); + UNPROTECT(nprotect); return(ansVec); } - From 837c901ed49c5fa338c8ed2176e013d283bc34db Mon Sep 17 00:00:00 2001 From: mattdowle Date: Wed, 23 May 2018 22:12:46 -0700 Subject: [PATCH 3/4] reorder.c --- src/reorder.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/reorder.c b/src/reorder.c index 6257d67e7b..4e7bb68af6 100644 --- a/src/reorder.c +++ b/src/reorder.c @@ -20,16 +20,20 @@ SEXP reorder(SEXP x, SEXP order) error("Column %d is length %d which differs from length of column 1 (%d). Invalid data.table.", i+1, length(v), nrow); if (SIZEOF(v) > maxSize) maxSize=SIZEOF(v); + if (ALTREP(v)) SET_VECTOR_ELT(x,i,duplicate(v)); // expand compact vector in place, ready for reordering by reference } } else { if (SIZEOF(x)!=4 && SIZEOF(x)!=8) error("reorder accepts vectors but this non-VECSXP is type '%s' which isn't yet supported", type2char(TYPEOF(x))); + if (ALTREP(x)) error("Internal error in reorder.c: cannot reorder an ALTREP vector. Please see NEWS item 2 in v1.11.4 and report this as a bug."); maxSize = SIZEOF(x); nrow = length(x); ncol = 1; } if (!isInteger(order)) error("order must be an integer vector"); if (length(order) != nrow) error("nrow(x)[%d]!=length(order)[%d]",nrow,length(order)); + int nprotect = 0; + if (ALTREP(order)) { order=PROTECT(duplicate(order)); nprotect++; } // TODO: how to fetch range of ALTREP compact vector R_len_t start = 0; while (start Date: Wed, 23 May 2018 22:14:09 -0700 Subject: [PATCH 4/4] news item 2 not 1 --- src/subset.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/subset.c b/src/subset.c index bc9e3884bb..3194433d6d 100644 --- a/src/subset.c +++ b/src/subset.c @@ -245,7 +245,7 @@ SEXP subsetDT(SEXP x, SEXP rows, SEXP cols) { SETLENGTH(ans, LENGTH(cols)); for (int i=0; i