From c4281b6839aef200464fdb5a565dcd0d416d73d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Budzy=C5=84ski?= Date: Mon, 2 Nov 2026 20:37:42 +0100 Subject: [PATCH 1/5] perf(unique): mix hash keys for sequential IDs The growable duplicate tables used the top bits of a multiplicative hash. Sequential integer IDs mapped to only about 16% of the slots in a power-of-two table, turning funique()/uniqLen()/fduplicated() into long linear-probe scans. Mix the key before indexing so the hash uses the table evenly. Equality checks and output order are unchanged. Add tools/benchmark-unique.R to keep the sequential-ID case visible. --- src/kit.h | 8 +++++++- tools/benchmark-unique.R | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 tools/benchmark-unique.R diff --git a/src/kit.h b/src/kit.h index 36f9736..d8f64ce 100644 --- a/src/kit.h +++ b/src/kit.h @@ -56,7 +56,13 @@ #define RCHAR(x, y) CHAR(STRING_ELT(x, y)) #define SEXP_F ScalarLogical(FALSE) #define SEXP_T ScalarLogical(TRUE) -#define HASH(key, K) (3141592653U * (unsigned int)(key) >> (32 - (K))) +static inline size_t kit_hash_key(uint32_t key, int K) { + // avalanche so sequential integer keys do not collide in power-of-two tables + key ^= key >> 16; + key *= 0x9E3779B1U; + return (size_t)(key >> (32 - K)); +} +#define HASH(key, K) kit_hash_key((uint32_t)(key), (K)) #define N_ISNAN(x, y) (!ISNAN(x) && !ISNAN(y)) #define B_IsNA(x, y) (R_IsNA(x) && R_IsNA(y)) #define B_IsNaN(x, y) (R_IsNaN(x) && R_IsNaN(y)) diff --git a/tools/benchmark-unique.R b/tools/benchmark-unique.R new file mode 100644 index 0000000..eb979c8 --- /dev/null +++ b/tools/benchmark-unique.R @@ -0,0 +1,34 @@ +args <- commandArgs(trailingOnly = TRUE) +if (length(args) > 4L) stop("Usage: Rscript tools/benchmark-unique.R [library] [label] [n] [unique]") +if (length(args) >= 1L && nzchar(args[[1]])) .libPaths(c(args[[1]], .libPaths())) +label <- if (length(args) >= 2L) args[[2]] else "current" +n <- if (length(args) >= 3L) as.integer(args[[3]]) else 10000000L +u <- if (length(args) >= 4L) as.integer(args[[4]]) else 100000L +library(kit) +options(kit.nThread = 1L) + +bench <- function(f, reps = 7L) { + invisible(f()) + times <- numeric(reps) + for (i in seq_len(reps)) { + invisible(gc()) + times[[i]] <- system.time(f())[["elapsed"]] + } + median(times) +} + +summarize <- function(name, f) { + cat(sprintf("%s\t%s\t%.6f\n", label, name, bench(f))) +} + +# Sequential IDs are the case that exposed the old top-bits-only hash: the +# table must still probe quickly when keys are contiguous, not just random. +x <- rep(seq_len(u), length.out = n) +stopifnot(identical(funique(x), seq_len(min(u, n)))) + +cat(sprintf("# R\t%s\tplatform\t%s\tpackage\t%s\tlabel\t%s\tn\t%d\tunique\t%d\n", + R.version.string, R.version$platform, + as.character(packageVersion("kit")), label, n, u)) +summarize("funique_seq", function() funique(x)) +summarize("uniqLen_seq", function() uniqLen(x)) +summarize("fduplicated_seq", function() fduplicated(x)) From 6ba0a465af14b5b48d9ae751b0d15ed0b394ca60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Budzy=C5=84ski?= Date: Mon, 2 Nov 2026 20:37:45 +0100 Subject: [PATCH 2/5] fix(nswitch): make duplicate values deterministic with threads Parallelise each atomic-output pair over the elements of x instead of parallelising the loop over pairs. With duplicate values, two pairs could previously write the same output slot from different threads, so the last matching pair was scheduling-dependent. The last pair now wins deterministically. Also fix the list-input/logical-output path, which indexed the output item by pair i instead of element j. --- inst/tinytest/test-nthread.R | 11 ++++++++ inst/tinytest/test-vswitch-nswitch.R | 11 ++++++++ src/nswitch.c | 42 ++++++++++++++-------------- 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/inst/tinytest/test-nthread.R b/inst/tinytest/test-nthread.R index 7c11e87..d866d7d 100644 --- a/inst/tinytest/test-nthread.R +++ b/inst/tinytest/test-nthread.R @@ -81,6 +81,17 @@ expect_identical( nswitch(xi, 1L, 10L, 2L, 20L, default = 0L, nThread = 2L), info="nthread-nswitch-002" ) +# duplicate values: last matching pair wins; the pair loop is serial so the +# result cannot depend on thread scheduling (issue #63 follow-up) +expect_identical( + nswitch(rep(1L, n), 1L, 10L, 1L, 20L, default = 0L, nThread = 1L), + nswitch(rep(1L, n), 1L, 10L, 1L, 20L, default = 0L, nThread = 2L), + info="nthread-nswitch-dup-001" +) +expect_true( + all(nswitch(rep(1L, n), 1L, 10L, 1L, 20L, default = 0L, nThread = 2L) == 20L), + info="nthread-nswitch-dup-002" +) # --- charToFact --- expect_identical( diff --git a/inst/tinytest/test-vswitch-nswitch.R b/inst/tinytest/test-vswitch-nswitch.R index a89e171..a15c704 100644 --- a/inst/tinytest/test-vswitch-nswitch.R +++ b/inst/tinytest/test-vswitch-nswitch.R @@ -915,6 +915,17 @@ expect_identical(vswitch(c(1L, 2L, 3L), c(1L, 1L, 2L), c(10L, 20L, 30L)), c(20L, 30L, NA_integer_), info="vswitch-0074.last-wins") expect_identical(vswitch(c(1L, 2L, 3L), c(1L, 1L, 2L), list(10L, 20L, 30L)), c(20L, 30L, NA_integer_), info="vswitch-0074.last-wins-list") +# issue #63 follow-up: duplicate nswitch values keep the last pair's result +expect_identical(nswitch(c(1L, 2L, 1L), 1L, 10L, 1L, 20L, default = 0L), + c(20L, 0L, 20L), info="nswitch-0074.last-wins-dup") +# list input + logical list output used the pair index instead of the element +# index when recycling vector outputs +expect_identical( + nswitch(list(1L, 1L, 2L), + list(1L), list(TRUE, FALSE, TRUE), + list(2L), list(FALSE, FALSE, TRUE), + default = list(FALSE)), + list(TRUE, FALSE, TRUE), info="nswitch-0074.list-output-index") expect_identical(vswitch(c(1L, 2L, 3L, NA_integer_), c(1L, 2L), c(10L, 20L), 99L), c(10L, 20L, 99L, 99L), info="vswitch-0074.default") expect_identical(vswitch(c(1L, 2L, 3L, 4L), c(1L, 2L), c(10L, 20L), 91:94), diff --git a/src/nswitch.c b/src/nswitch.c index aa0a7e1..c336780 100644 --- a/src/nswitch.c +++ b/src/nswitch.c @@ -148,10 +148,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { switch(type_x) { case LGLSXP: { const int *restrict px = LOGICAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = LOGICAL(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = LOGICAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -161,10 +161,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case INTSXP: { const int *restrict px = INTEGER(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = LOGICAL(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = INTEGER(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -174,10 +174,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case REALSXP: { const double *restrict px = REAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = LOGICAL(PTR_ETL(args, 2*i+1)); const double *restrict pvalues = REAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -187,10 +187,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case CPLXSXP: { const Rcomplex *restrict px = COMPLEX(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = LOGICAL(PTR_ETL(args, 2*i+1)); const Rcomplex *restrict pvalues = COMPLEX(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (EQUAL_CPLX(px[j], pvalues[0])) { pans[j] = pto[j & amask[i]]; @@ -200,10 +200,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case STRSXP: { const SEXP *restrict px = STRING_PTR_RO(utfcon ? xans : x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = LOGICAL(PTR_ETL(args, 2*i+1)); const SEXP *restrict pvalues = STRING_PTR_RO(utfcon ? vans : PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -218,7 +218,7 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { const SEXP *restrict pvalues = SEXPPTR_RO(PTR_ETL(args, 2*i)); for (ssize_t j = 0; j < len_x; ++j) { if (R_compute_identical(px[j],pvalues[0],0)) { - pans[j] = pto[i & amask[i]]; + pans[j] = pto[j & amask[i]]; } } } @@ -241,10 +241,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { switch(type_x) { case LGLSXP: { const int *restrict px = LOGICAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = INTEGER(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = LOGICAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -254,10 +254,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case INTSXP: { const int *restrict px = INTEGER(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = INTEGER(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = INTEGER(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -267,10 +267,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case REALSXP: { const double *restrict px = REAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = INTEGER(PTR_ETL(args, 2*i+1)); const double *restrict pvalues = REAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -280,10 +280,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case CPLXSXP: { const Rcomplex *restrict px = COMPLEX(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = INTEGER(PTR_ETL(args, 2*i+1)); const Rcomplex *restrict pvalues = COMPLEX(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (EQUAL_CPLX(px[j], pvalues[0])) { pans[j] = pto[j & amask[i]]; @@ -293,10 +293,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case STRSXP: { const SEXP *restrict px = STRING_PTR_RO(utfcon ? xans : x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const int *restrict pto = INTEGER(PTR_ETL(args, 2*i+1)); const SEXP *restrict pvalues = STRING_PTR_RO(utfcon ? vans : PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -334,10 +334,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { switch(type_x) { case LGLSXP: { const int *restrict px = LOGICAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const double *restrict pto = REAL(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = LOGICAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -347,10 +347,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case INTSXP: { const int *restrict px = INTEGER(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const double *restrict pto = REAL(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = INTEGER(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -360,10 +360,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case REALSXP: { const double *restrict px = REAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const double *restrict pto = REAL(PTR_ETL(args, 2*i+1)); const double *restrict pvalues = REAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -373,10 +373,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case CPLXSXP: { const Rcomplex *restrict px = COMPLEX(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const double *restrict pto = REAL(PTR_ETL(args, 2*i+1)); const Rcomplex *restrict pvalues = COMPLEX(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (EQUAL_CPLX(px[j], pvalues[0])) { pans[j] = pto[j & amask[i]]; @@ -386,10 +386,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case STRSXP: { const SEXP *restrict px = STRING_PTR_RO(utfcon ? xans : x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const double *restrict pto = REAL(PTR_ETL(args, 2*i+1)); const SEXP *restrict pvalues = STRING_PTR_RO(utfcon ? vans : PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -428,10 +428,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { switch(type_x) { case LGLSXP: { const int *restrict px = LOGICAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const Rcomplex *restrict pto = COMPLEX(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = LOGICAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -441,10 +441,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case INTSXP: { const int *restrict px = INTEGER(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const Rcomplex *restrict pto = COMPLEX(PTR_ETL(args, 2*i+1)); const int *restrict pvalues = INTEGER(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -454,10 +454,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case REALSXP: { const double *restrict px = REAL(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const Rcomplex *restrict pto = COMPLEX(PTR_ETL(args, 2*i+1)); const double *restrict pvalues = REAL(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; @@ -467,10 +467,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case CPLXSXP: { const Rcomplex *restrict px = COMPLEX(x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const Rcomplex *restrict pto = COMPLEX(PTR_ETL(args, 2*i+1)); const Rcomplex *restrict pvalues = COMPLEX(PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (EQUAL_CPLX(px[j],pvalues[0])) { pans[j] = pto[j & amask[i]]; @@ -480,10 +480,10 @@ SEXP nswitchR(SEXP x, SEXP na, SEXP nthreads, SEXP chkenc, SEXP args) { } break; case STRSXP: { const SEXP *restrict px = STRING_PTR_RO(utfcon ? xans : x); - OMP_PARALLEL_FOR(nth) for (ssize_t i = 0; i < n; ++i) { const Rcomplex *restrict pto = COMPLEX(PTR_ETL(args, 2*i+1)); const SEXP *restrict pvalues = STRING_PTR_RO(utfcon ? vans : PTR_ETL(args, 2*i)); + OMP_PARALLEL_FOR(nth) for (ssize_t j = 0; j < len_x; ++j) { if (px[j] == pvalues[0]) { pans[j] = pto[j & amask[i]]; From 1e7a3ba4bc51c892d43029e758e425dbc1947bd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Budzy=C5=84ski?= Date: Mon, 2 Nov 2026 20:37:48 +0100 Subject: [PATCH 3/5] fix(long vectors): guard logical fduplicated and class topn fduplicated() used an int loop index for logical vectors and stored i + 1 in an int table slot, so a logical vector above INT_MAX elements could read out of bounds. Refuse it with the standard long-vector error, matching funique() and uniqLen(). Apply xtfrm() before topn()'s long-vector heap path so classed long inputs follow order() semantics there too. Error clearly when xtfrm() does not return an integer/double vector of the requested length, and cover the cheap ALTREP error path in the long-vector tests. --- inst/tinytest/test-long-vectors.R | 12 ++++++++++++ man/topn.Rd | 10 +++++----- src/dup.c | 6 ++++++ src/topn.c | 32 ++++++++++++++++++------------- 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/inst/tinytest/test-long-vectors.R b/inst/tinytest/test-long-vectors.R index 768d9aa..5bcca71 100644 --- a/inst/tinytest/test-long-vectors.R +++ b/inst/tinytest/test-long-vectors.R @@ -33,6 +33,16 @@ expect_identical(funique(seq_len(10000L)), seq_len(10000L), info = "longv-0001.0 expect_identical(uniqLen(seq_len(10000L)), 10000L, info = "longv-0001.007") expect_identical(topn(seq_len(10000L), 5L, decreasing = FALSE, hasna = FALSE), 1:5, info = "longv-0001.008") +# classed long vectors must not silently take the integer-index order() path: +# the xtfrm result is checked before selection and a non-numeric/short result +# errors. seq_len() is a compact ALTREP sequence, so this allocates nothing. +nlong <- 2^31 + 1000000 +xl <- structure(seq_len(nlong), class = "kit_topn_long_bad") +registerS3method("xtfrm", "kit_topn_long_bad", function(x) 1L) +expect_error(topn(xl, 2L), pattern = "Classed long vectors are not supported by 'topn()'.", + fixed = TRUE, info = "longv-0001.126") +rm(xl, nlong) + # a data.frame with > INT_MAX cells is refused up front. All columns share one # small logical vector, so building it is cheap and the error fires before # dfToMatrix allocates the matrix. @@ -93,6 +103,8 @@ invisible(gc()) lg <- rep(TRUE, n) expect_identical(funique(lg), TRUE, info = "longv-0001.106") expect_identical(uniqLen(lg), 1L, info = "longv-0001.107") +expect_error(fduplicated(lg), pattern = "Long vector not supported yet", + fixed = TRUE, info = "longv-0001.125") rm(lg) invisible(gc()) diff --git a/man/topn.Rd b/man/topn.Rd index be54824..2c1d24c 100644 --- a/man/topn.Rd +++ b/man/topn.Rd @@ -37,16 +37,16 @@ topn(vec, n=6L, decreasing=TRUE, hasna=TRUE, index=TRUE) For inputs longer than \code{.Machine$integer.max}, \code{topn()} keeps a heap of at most \code{n} entries instead, so memory stays proportional to - \code{n} (about 24 bytes per entry plus 8 bytes per returned index). Class - attributes are ignored and the stored values are compared. Unlike + \code{n} (about 24 bytes per entry plus 8 bytes per returned index). Unlike \code{base::order()}, \code{decreasing = TRUE} works for these inputs. For \code{n} above roughly three quarters of the input length this heap can use more memory than a full-length \code{order()} index, and \code{order()} is faster for \code{decreasing = FALSE}. - For classed numeric vectors shorter than \code{.Machine$integer.max}, - \code{xtfrm()} is applied once before selection, so their order matches - \code{order()}/\code{xtfrm()}. + For classed numeric vectors, \code{xtfrm()} is applied once before + selection, so their order matches \code{order()}/\code{xtfrm()}. This also + applies to long inputs; a classed long vector whose \code{xtfrm()} result is + not integer or double errors. } \value{ If \code{index = TRUE}, an integer vector of the indices of the \code{n} diff --git a/src/dup.c b/src/dup.c index 1acc926..aafb51f 100644 --- a/src/dup.c +++ b/src/dup.c @@ -964,6 +964,9 @@ SEXP dupVecR(SEXP x, SEXP uniq, SEXP fromLast) { Kbase++; } } else if (tx == LGLSXP) { + if (n > INT_MAX) { + error("Length of 'x' is too large. (Long vector not supported yet)"); + } M = 4; K = 2; Mbase = 4; @@ -1429,6 +1432,9 @@ SEXP dupVecIndexOnlyR(SEXP x) { Kbase++; } } else if (tx == LGLSXP) { + if (n > INT_MAX) { + error("Length of 'x' is too large. (Long vector not supported yet)"); + } M = 4; K = 2; Mbase = 4; diff --git a/src/topn.c b/src/topn.c index 9050468..c0b9a04 100644 --- a/src/topn.c +++ b/src/topn.c @@ -212,19 +212,11 @@ SEXP topnR(SEXP vec, SEXP n, SEXP dec, SEXP hasna, SEXP env) { const bool dcr = asLogical(dec); SEXPTYPE tvec = UTYPEOF(vec); bool vhasna = asLogical(hasna); - // len1 > INT_MAX: ans is INTSXP and cannot hold indices into a long vector; - // the bounded-heap selection below returns double indices like base::order() - // with missing values last (order(na.last = TRUE) semantics) and uses memory - // proportional to n instead of a full-length order() index (~8*len1 bytes, - // ~17 GB for 2^31 elements) (#83). Unlike order(), it also supports - // decreasing = TRUE, which errors on long vectors in base R. - if (len1 > INT_MAX && (tvec == INTSXP || tvec == REALSXP)) { - return topnLong(vec, len0, dcr); - } - // Classed vectors must follow order()/xtfrm on every non-long path. Apply - // xtfrm once and keep the fast selector: for identity methods (Date, - // POSIXct, factor) this avoids sorting the whole vector just to return a - // few indices, and it still matches order() for custom classes. + // Classed vectors must follow order()/xtfrm on every path, including the + // long-vector heap below. Apply xtfrm once and keep the fast selector: for + // identity methods (Date, POSIXct, factor) this avoids sorting the whole + // vector just to return a few indices, and it still matches order() for + // custom classes. int nprotect = 0; if (getAttrib(vec, R_ClassSymbol) != R_NilValue && (tvec == INTSXP || tvec == REALSXP)) { @@ -238,9 +230,23 @@ SEXP topnR(SEXP vec, SEXP n, SEXP dec, SEXP hasna, SEXP env) { nprotect = 2; } else { UNPROTECT(2); + if (len1 > INT_MAX) { + error("Classed long vectors are not supported by 'topn()'."); + } return topnRadix(vec, len0, dcr); } } + // len1 > INT_MAX: ans is INTSXP and cannot hold indices into a long vector; + // the bounded-heap selection below returns double indices like base::order() + // with missing values last (order(na.last = TRUE) semantics) and uses memory + // proportional to n instead of a full-length order() index (~8*len1 bytes, + // ~17 GB for 2^31 elements) (#83). Unlike order(), it also supports + // decreasing = TRUE, which errors on long vectors in base R. + if (len1 > INT_MAX && (tvec == INTSXP || tvec == REALSXP)) { + SEXP ans = topnLong(vec, len0, dcr); + UNPROTECT(nprotect); + return ans; + } if ( ((len0 > 2000 && !vhasna) || (len0 > 1500 && vhasna)) && (tvec == INTSXP || tvec == REALSXP)) { SEXP ans = topnRadix(vec, len0, dcr); UNPROTECT(nprotect); From e42b5162d0711a95edfb0b36f6433f158de5b8b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Budzy=C5=84ski?= Date: Mon, 2 Nov 2026 20:37:51 +0100 Subject: [PATCH 4/5] docs(news): prepare 0.0.23 release notes --- NEWS.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 60d4f35..a043d2b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,13 @@ -# kit (development version) +# kit 0.0.23 (2026-11-02) ### Bug Fixes +- Fix `nswitch()` with duplicate values and `nThread > 1`: the outer loop over value/output pairs is now serial and atomic-output pairs are parallelised over the elements of `x`, so the last matching pair wins deterministically. Also fix the list-input/logical-output path reading output element `i` instead of `j` (#63 follow-up). + +- Fix `fduplicated()` on logical vectors longer than `INT_MAX`: the generic path used an `int` loop index and stored `i + 1` in an `int` table slot, so a multi-gigabyte logical vector could overflow and read out of bounds. It now raises the standard "Long vector not supported yet" error (#61 follow-up). + +- Apply `xtfrm` to classed vectors before the `topn()` long-vector heap path, so custom classes follow `order()` semantics there too; classed long vectors whose `xtfrm` is not integer/double now error clearly instead of falling back to the integer-index `order()` path (#83 follow-up). + - Fix `nswitch()` segfaulting when called without value/output pairs (e.g. `nswitch(1:3)` or `nswitch(1:3, default = 0L)`): the argument count is now validated before any pair is read, and the call errors with a clear message (#87). - Evaluate the internal `order()`/`sort()` calls in `topn`, `psort`, and `charToFact` in the base environment. A caller-defined `order`/`sort` could previously shadow them and make `topn` return out-of-bounds indices or `psort`/`charToFact` error or sort incorrectly (#74). @@ -24,6 +30,8 @@ ### Notes +- Mix hashed keys with a 32-bit avalanche before indexing the growable duplicate tables. The previous top-bits-only multiplicative hash mapped sequential integer IDs to only ~16% of the slots in a power-of-two table, so `funique()`/`fduplicated()`/`uniqLen()` fell back to long linear-probe scans on common ID-like data. The tables still start at 256 slots and grow on demand; adding `tools/benchmark-unique.R` keeps this case visible. + - Document `?kit` with a package page that explains OpenMP threads, platform support, encoding behavior, and the main functions. Add "OpenMP and threads" and "Platforms" sections to the README, and add `library(kit)` to the first README example (#90). - Reduce peak memory in `funique`, `fduplicated`, `uniqLen`, `psort`, and `charToFact` by starting duplicate hash tables at 256 slots and growing them on demand (4x, capped at the previous full-size table) instead of allocating the full table up front. Rehashing re-reads the source values, so results are unchanged; inputs with few unique values use much less memory, while peak memory can be higher at growth boundaries for high-cardinality inputs (#74). From 25a2a552ced415d97148678ec80cdcb8cadeeb42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Budzy=C5=84ski?= Date: Mon, 2 Nov 2026 21:11:52 +0100 Subject: [PATCH 5/5] chore(coverage): exclude long-vector-only branches The topn long-heap path scans more than INT_MAX elements, and the logical fduplicated guards need an 8 GB vector. Both are covered only by the opt-in KIT_TEST_LONG_VECTOR suite, so mark them # nocov like the other long-vector guards. --- src/dup.c | 4 ++-- src/topn.c | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/dup.c b/src/dup.c index aafb51f..818cc28 100644 --- a/src/dup.c +++ b/src/dup.c @@ -965,7 +965,7 @@ SEXP dupVecR(SEXP x, SEXP uniq, SEXP fromLast) { } } else if (tx == LGLSXP) { if (n > INT_MAX) { - error("Length of 'x' is too large. (Long vector not supported yet)"); + error("Length of 'x' is too large. (Long vector not supported yet)"); // # nocov } M = 4; K = 2; @@ -1433,7 +1433,7 @@ SEXP dupVecIndexOnlyR(SEXP x) { } } else if (tx == LGLSXP) { if (n > INT_MAX) { - error("Length of 'x' is too large. (Long vector not supported yet)"); + error("Length of 'x' is too large. (Long vector not supported yet)"); // # nocov } M = 4; K = 2; diff --git a/src/topn.c b/src/topn.c index c0b9a04..ff8f0ab 100644 --- a/src/topn.c +++ b/src/topn.c @@ -243,9 +243,10 @@ SEXP topnR(SEXP vec, SEXP n, SEXP dec, SEXP hasna, SEXP env) { // ~17 GB for 2^31 elements) (#83). Unlike order(), it also supports // decreasing = TRUE, which errors on long vectors in base R. if (len1 > INT_MAX && (tvec == INTSXP || tvec == REALSXP)) { - SEXP ans = topnLong(vec, len0, dcr); - UNPROTECT(nprotect); - return ans; + // only exercised by KIT_TEST_LONG_VECTOR: the heap scan is O(len1) + SEXP ans = topnLong(vec, len0, dcr); // # nocov + UNPROTECT(nprotect); // # nocov + return ans; // # nocov } if ( ((len0 > 2000 && !vhasna) || (len0 > 1500 && vhasna)) && (tvec == INTSXP || tvec == REALSXP)) { SEXP ans = topnRadix(vec, len0, dcr);