Skip to content

Added an atime test for performance improvement in forderv when reusing existing key and index attributes #6555

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
merged 15 commits into from
Oct 11, 2024
Merged

Added an atime test for performance improvement in forderv when reusing existing key and index attributes #6555

Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5a15dd2
Added the test with minor changes to comments
Anirban166 Oct 3, 2024
7e822ad
Narrow down on N
Anirban166 Oct 3, 2024
4ecb240
Go with default N spec
Anirban166 Oct 3, 2024
b0c10f5
Set seed for reproducibility given the use of sample()
Anirban166 Oct 3, 2024
ba72deb
rm sort reuse option
Anirban166 Oct 3, 2024
c519de1
for loop over retGrp=T,F
Oct 3, 2024
f24984a
historical sha1s from commits page instead of merge commit
Oct 3, 2024
6e69305
rm > from test name since that fails on CI
Oct 3, 2024
401f3d7
remove extra blank lines
Oct 3, 2024
7800a62
use setattr-> Fast always linear
Oct 3, 2024
7a16341
forderv retGrp=T or F, no setindex
tdhock Oct 8, 2024
6603d06
more detailed comment about expectations
tdhock Oct 8, 2024
6509080
restore old option after test
tdhock Oct 8, 2024
4e69115
log
tdhock Oct 8, 2024
13c4b1f
undo white space addition
tdhock Oct 11, 2024
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
30 changes: 29 additions & 1 deletion .ci/atime/tests.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,34 @@ for (extra.arg in extra.args.6107){
extra.test.list[[sprintf("fread(%s) improved in #6107", extra.arg)]] <- this.test
}

# Test case adapted from https://github.com/Rdatatable/data.table/pull/4386#issue-602528139 which is where the performance was improved.
retGrp_values <- c("T","F")
for(retGrp_setup in retGrp_values){
for(retGrp_expr in retGrp_values){
test.name <- sprintf("forderv(retGrp=%s-%s) improved in #4386", retGrp_setup, retGrp_expr)
extra.test.list[[test.name]] <- list(
setup = quote({
options(datatable.forder.auto.index = TRUE)
set.seed(1)
dt <- data.table(index = sample(N), values = sample(N))
index.list <- list()
for(retGrp in retGrp_values){
data.table:::forderv(dt, "index", retGrp = eval(str2lang(retGrp)))
index.list[[retGrp]] <- attr(dt, "index")
}
}),
expr = substitute({
setattr(dt, "index", index.list[[retGrp_setup]])
data.table:::forderv(dt, "index", retGrp = retGrp_expr) # Reusing the index and computing group info.
}, list(
retGrp_setup=retGrp_setup,
retGrp_expr=str2lang(retGrp_expr)
)),
Slow = "b1b1832b0d2d4032b46477d9fe6efb15006664f4", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/b0efcf59442a7d086c6df17fa6a45c81b082322e) in the PR (https://github.com/Rdatatable/data.table/pull/4386/commits) where the performance was improved.
Fast = "ffe431fbc1fe2d52ed9499f78e7e16eae4d71a93") # Last commit of the PR (https://github.com/Rdatatable/data.table/pull/4386/commits) where the performance was improved.
}
}

# A list of performance tests.
#
# See documentation in https://github.com/Rdatatable/data.table/wiki/Performance-testing for best practices.
Expand Down Expand Up @@ -207,6 +235,6 @@ test.list <- atime::atime_test_list(
expr = data.table:::melt(DT, measure.vars = measure.vars),
Slow = "fd24a3105953f7785ea7414678ed8e04524e6955", # Parent of the merge commit (https://github.com/Rdatatable/data.table/commit/ed72e398df76a0fcfd134a4ad92356690e4210ea) of the PR (https://github.com/Rdatatable/data.table/pull/5054) that fixes the issue
Fast = "ed72e398df76a0fcfd134a4ad92356690e4210ea"), # Merge commit of the PR (https://github.com/Rdatatable/data.table/pull/5054) that fixes the issue

tests=extra.test.list)
# nolint end: undesirable_operator_linter.
Loading