diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 2d342ded67..4b9e6ea92e 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -459,5 +459,23 @@ test.list <- atime::atime_test_list( seconds.limit = 1, expr = data.table:::`[.data.table`(dt, , base::max(V1, na.rm = TRUE), by = id)), + # https://github.com/Rdatatable/data.table/pull/7917 creates the variable string once per measure group instead of once per row. + "melt(list measure.vars, variable.factor=FALSE) improved in #7917" = atime::atime_test( + setup = { + DT = data.table(id = seq_len(N), a_1 = 1, a_2 = 2, b_1 = 3, b_2 = 4) + }, + Slow = "df59dd637403e65d7fa90d48bb0dfec2764d6744", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/7c73bf0d4f760066feb7256896375727656e398d) in the PR (https://github.com/Rdatatable/data.table/pull/7917/commits) that improves efficiency + Fast = "7c73bf0d4f760066feb7256896375727656e398d", # Commit in the PR (https://github.com/Rdatatable/data.table/pull/7917/commits) that creates the variable string once per measure group TODO after merging PR7917, change to its merge commit + expr = data.table:::melt(DT, measure.vars = list(c("a_1", "a_2"), c("b_1", "b_2")), variable.factor = FALSE)), + + # https://github.com/Rdatatable/data.table/pull/7917 hoists INTEGER()/STRING_ELT() out of the row loop when filling variable_table columns. + "melt(measure()) improved in #7917" = atime::atime_test( + setup = { + DT = data.table(id = seq_len(N), a_1 = 1, a_2 = 2, b_1 = 3, b_2 = 4) + }, + Slow = "df59dd637403e65d7fa90d48bb0dfec2764d6744", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/7c73bf0d4f760066feb7256896375727656e398d) in the PR (https://github.com/Rdatatable/data.table/pull/7917/commits) that improves efficiency + Fast = "45c63fa0410c441cbc3764d3fa19b47344d0e418", # Commit in the PR (https://github.com/Rdatatable/data.table/pull/7917/commits) that hoists REAL()/INTEGER()/STRING_ELT() out of the row loop TODO after merging PR7917, change to its merge commit + expr = data.table:::melt(DT, measure.vars = measure(letter, number = as.integer))), + tests=extra.test.list) # nolint end: undesirable_operator_linter. diff --git a/NEWS.md b/NEWS.md index ee2f96fd97..cb4a6231fb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -48,6 +48,8 @@ 13. `nafill()` and `setnafill()` gain a `limit` argument to restrict the maximum number of consecutive `NA` values filled, [#7677](https://github.com/Rdatatable/data.table/issues/7677). Thanks to @jaynewton for the suggestion and @venom1204 and @ben-schwen for the PR. +14. `melt()` is faster on large inputs when `measure.vars` is a list with `variable.factor=FALSE` (e.g. 3x) or uses `measure()` (e.g. 2x). Thanks @m-muecke for the PR. + ### BUG FIXES 1. `fread()` with `skip=0` and `(header=TRUE|FALSE)` no longer skips the first row when it has fewer fields than subsequent rows, [#7463](https://github.com/Rdatatable/data.table/issues/7463). Thanks @emayerhofer for the report and @ben-schwen for the fix. diff --git a/src/fmelt.c b/src/fmelt.c index 7db6b09916..fc7235febf 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -611,7 +611,8 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; char buff[20]; snprintf(buff, sizeof(buff), "%d", level++); // # notranslate - for (int k=0; klmax; ++j) { const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; switch (TYPEOF(target)) { - case STRSXP : + case STRSXP : { + const SEXP val = STRING_ELT(out_col, j); for (int k=0; kvariable_table, R_NamesSymbol), out_col_i))); }