Skip to content

Commit 07c1e67

Browse files
committed
eke out small gains with optimized conditionals, subsetting, and loops
1 parent 18d3ad7 commit 07c1e67

File tree

6 files changed

+51
-23
lines changed

6 files changed

+51
-23
lines changed

Diff for: R/class_builder.R

+4-4
Original file line numberDiff line numberDiff line change
@@ -397,10 +397,10 @@ builder_ensure_workspace <- function(target, pipeline, scheduler, meta) {
397397

398398
builder_should_save_workspace <- function(target) {
399399
names <- c(target_get_name(target), target_get_parent(target))
400-
because_named <- any(names %in% tar_options$get_workspaces())
401-
has_error <- metrics_has_error(target$metrics)
402-
if_error <- tar_options$get_workspace_on_error() ||
403-
identical(target$settings$error, "workspace")
400+
because_named <- any(names %in% .subset2(tar_options, "workspaces"))
401+
has_error <- metrics_has_error(.subset2(target, "metrics"))
402+
if_error <- .subset2(tar_options, "get_workspace_on_error")() ||
403+
identical(.subset2(.subset2(target, "settings"), "error"), "workspace")
404404
because_error <- if_error && has_error
405405
because_named || because_error
406406
}

Diff for: R/class_counter.R

+4-3
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,10 @@ counter_set_names <- function(counter, names) {
8080
}
8181

8282
counter_del_name <- function(counter, name) {
83-
if (counter_exists_name(counter, name)) {
84-
remove(list = name, envir = counter$envir)
85-
counter$count <- counter$count - 1L
83+
envir <- .subset2(counter, "envir")
84+
if (!is.null(.subset2(envir, name))) {
85+
remove(list = name, envir = envir)
86+
counter$count <- .subset2(counter, "count") - 1L
8687
}
8788
}
8889

Diff for: R/class_database.R

+11-5
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,15 @@ database_class <- R6::R6Class(
183183
if (fill_missing) {
184184
row <- select_cols(row)
185185
}
186-
line <- produce_line(row)
187-
buffer[[.subset2(row, "name")]] <- line
186+
sublines <- produce_sublines(row)
187+
buffer[[.subset2(row, "name")]] <- sublines
188188
self$buffer_length <- buffer_length + 1L
189189
},
190190
flush_rows = function() {
191-
if (buffer_length) {
192-
append_lines(as.character(as.list(buffer)))
191+
if (buffer_length > 0L) {
192+
lines_list <- eapply(buffer, paste, collapse = database_sep_outer)
193+
lines <- as.character(lines_list)
194+
append_lines(lines)
193195
self$buffer <- new.env(parent = emptyenv(), hash = FALSE)
194196
self$buffer_length <- 0L
195197
self$staged <- TRUE
@@ -255,7 +257,7 @@ database_class <- R6::R6Class(
255257
)
256258
file_move(from = tmp, to = self$path)
257259
},
258-
produce_line = function(row) {
260+
produce_sublines = function(row) {
259261
old <- options(OutDec = ".")
260262
on.exit(options(old))
261263
index <- 1L
@@ -265,6 +267,10 @@ database_class <- R6::R6Class(
265267
sublines[index] <- produce_subline(.subset2(row, index))
266268
index <- index + 1L
267269
}
270+
sublines
271+
},
272+
produce_line = function(row) {
273+
sublines <- produce_sublines(row)
268274
paste(sublines, collapse = database_sep_outer)
269275
},
270276
produce_subline = function(element) {

Diff for: R/class_file.R

+29-8
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,22 @@ file_update_hash <- function(file) {
7979
}
8080

8181
file_should_rehash <- function(file, time, size, trust_timestamps) {
82-
if_any(
83-
.subset2(tar_options, "trust_timestamps") %|||% trust_timestamps,
84-
!identical(time, file$time) || !identical(size, file$size),
85-
TRUE
86-
)
82+
trust <- .subset2(tar_options, "trust_timestamps")
83+
if (is.null(trust)) {
84+
trust <- trust_timestamps
85+
}
86+
if (trust) {
87+
file_time <- .subset2(file, "time")
88+
file_size <- .subset2(file, "size")
89+
if (anyNA(file_time) || anyNA(file_size)) {
90+
out <- TRUE
91+
} else {
92+
out <- (time != file_time) || (size != file_size)
93+
}
94+
} else {
95+
out <- TRUE
96+
}
97+
out
8798
}
8899

89100
file_repopulate <- function(file, path, data) {
@@ -111,7 +122,7 @@ file_ensure_hash <- function(file) {
111122
}
112123

113124
file_has_correct_hash <- function(file) {
114-
files <- file_list_files(file$path)
125+
files <- file_list_files(.subset2(file, "path"))
115126
info <- file_info_runtime(files)
116127
time <- file_time(info)
117128
bytes <- file_bytes(info)
@@ -120,9 +131,19 @@ file_has_correct_hash <- function(file) {
120131
file = file,
121132
time = time,
122133
size = size,
123-
trust_timestamps = all(info$trust_timestamps)
134+
trust_timestamps = all(.subset2(info, "trust_timestamps"))
124135
)
125-
if_any(do, identical(file$hash, file_hash(files)), TRUE)
136+
if (do) {
137+
file_hash <- .subset2(file, "hash")
138+
if (anyNA(file_hash)) {
139+
out <- FALSE
140+
} else {
141+
out <- file_hash == file_hash(files)
142+
}
143+
} else {
144+
out <- TRUE
145+
}
146+
out
126147
}
127148

128149
file_validate_path <- function(path) {

Diff for: R/class_metrics.R

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ metrics_has_warnings <- function(metrics) {
2121
}
2222

2323
metrics_has_error <- function(metrics) {
24-
!is.null(metrics$error)
24+
!is.null(.subset2(metrics, "error"))
2525
}
2626

2727
metrics_has_cancel <- function(metrics) {

Diff for: tests/testthat/test-class_database.R

+2-2
Original file line numberDiff line numberDiff line change
@@ -398,10 +398,10 @@ tar_test("database buffer", {
398398
expect_false(file.exists(db$path))
399399
db$flush_rows()
400400
lines <- readLines(db$path)
401-
expect_equal(lines, c("x", "y"))
401+
expect_equal(sort(lines), sort(c("x", "y")))
402402
db$flush_rows()
403403
lines <- readLines(db$path)
404-
expect_equal(lines, c("x", "y"))
404+
expect_equal(sort(lines), sort(c("x", "y")))
405405
})
406406

407407
tar_test("compare_working_directories()", {

0 commit comments

Comments
 (0)