Skip to content

perf(core): optimize nest_for_ard group partitioning, process_nested_list, and batch apply_fmt_fun - #610

Open
kpagacz wants to merge 2 commits into
pharmaverse:mainfrom
kpagacz:main
Open

perf(core): optimize nest_for_ard group partitioning, process_nested_list, and batch apply_fmt_fun#610
kpagacz wants to merge 2 commits into
pharmaverse:mainfrom
kpagacz:main

Conversation

@kpagacz

@kpagacz kpagacz commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Flamegraph profiling of cards::ard_summary() and cards::ard_missing() revealed following possible performance gains:

  1. cards::nest_for_ard() dynamically generated quosures and executed dplyr::filter() and dplyr::select() G times across all N observations (O(G * N) complexity), repeatedly scanning the dataset and evaluating tidyselect column specifications.
  2. .process_nested_list_as_df() executed per-variable dplyr::filter() followed by dplyr::rows_update(), forcing hash key table validation and full dataframe copies.
  3. cards::apply_fmt_fun() performed row-by-row purrr::pmap() iteration, resolving function aliases, creating closures via label_round(), and executing scalar format() calls independently for each cell (!!).

Changes:

  1. cards/R/nest_for_ard.R:

    • Replaced dynamic quosure filtering loop with O(N) integer index matching via vctrs::vec_match() against the unique group combination grid.
    • Sliced row subsets directly using base R matrix/dataframe indexing on pre-split integer indices, refactoring out dplyr::filter() and tidyselect evaluations per stratum.
    • Replaced mutate(across(..., as.list)) with direct in-place list column wrapping.
  2. cards/R/ard_summary.R:

    • Refactored .process_nested_list_as_df():
      • Extracted unique (variable, stat_name) pairs upfront and grouped into a lookup list.
      • Calculated formula selectors per variable and computed 1-to-1 integer indices via vctrs::vec_match().
      • Replaced dplyr::rows_update() with direct positional assignment x[[new_column]][idx] <- vals.
  3. cards/R/apply_fmt_fun.R:

    • Grouped rows sharing identical fmt_fun definitions and resolved function aliases once per group rather than per row.
    • Vectorized formatting across statistic vectors for supported primitive and standard rounding functions, falling back to scalar evaluations only when required.
    • Maintained full condition capture parity (stat_fmt, warning, error) matching existing ARD S3 structure.

What changes are proposed in this pull request?

  • optimize nest_for_ard group partitioning, process_nested_list, and batch apply_fmt_fun, @kpagacz

Reference GitHub issue associated with pull request. e.g., 'closes #'


Pre-review Checklist (if item does not apply, mark is as complete)

  • All GitHub Action workflows pass with a ✅
  • PR branch has pulled the most recent updates from master branch: usethis::pr_merge_main()
  • If a bug was fixed, a unit test was added.
  • Code coverage is suitable for any new functions/features (generally, 100% coverage for new code): devtools::test_coverage()
  • Request a reviewer

Reviewer Checklist (if item does not apply, mark is as complete)

  • If a bug was fixed, a unit test was added.
  • Run pkgdown::build_site(). Check the R console for errors, and review the rendered website.
  • Code coverage is suitable for any new functions/features: devtools::test_coverage()

When the branch is ready to be merged:

  • Update NEWS.md with the changes from this pull request under the heading "# cards (development version)". If there is an issue associated with the pull request, reference it in parentheses at the end update (see NEWS.md for examples).
  • All GitHub Action workflows pass with a ✅
  • Approve Pull Request
  • Merge the PR. Please use "Squash and merge" or "Rebase and merge".

Optional Reverse Dependency Checks:

Install checked with pak::pak("Genentech/checked") or pak::pak("checked")

# Check dev versions of `cardx`, `gtsummary`, and `tfrmt` which are in the `ddsjoberg` R Universe
Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2L, repos = c('https://ddsjoberg.r-universe.dev', 'https://cloud.r-project.org'))"

# Check CRAN reverse dependencies but run tests skipped on CRAN
Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')"

# Check CRAN reverse dependencies in a CRAN-like environment
Rscript -e "options(checked.check_envvars = c(NOT_CRAN = FALSE), checked.check_build_args = '--as-cran'); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')"

…list, and batch apply_fmt_fun

Flamegraph profiling of cards::ard_summary() and cards::ard_missing() revealed
critical allocation and CPU bottlenecks during data partitioning, parameter updates,
and statistic formatting:
1. cards::nest_for_ard() dynamically generated quosures and executed dplyr::filter()
   and dplyr::select() G times across all N observations (O(G * N) complexity), repeatedly
   scanning the dataset and evaluating tidyselect column specifications.
2. .process_nested_list_as_df() executed per-variable dplyr::filter() followed by
   dplyr::rows_update(), forcing hash key table validation and full dataframe copies.
3. cards::apply_fmt_fun() performed row-by-row purrr::pmap() iteration, resolving
   function aliases, creating closures via label_round(), and executing scalar format()
   calls independently for each cell.

Changes:

1. cards/R/nest_for_ard.R:
   - Replaced dynamic quosure filtering loop with O(N) integer index matching via
     vctrs::vec_match() against the unique group combination grid.
   - Sliced row subsets directly using base R matrix/dataframe indexing on pre-split
     integer indices, bypassing dplyr::filter() and tidyselect evaluations per stratum.
   - Replaced mutate(across(..., as.list)) with direct in-place list column wrapping.

2. cards/R/ard_summary.R:
   - Refactored .process_nested_list_as_df():
     * Extracted unique (variable, stat_name) pairs upfront and grouped into a lookup list.
     * Calculated formula selectors per variable and computed 1-to-1 integer indices via
       vctrs::vec_match().
     * Replaced dplyr::rows_update() with direct positional assignment x[[new_column]][idx] <- vals.

3. cards/R/apply_fmt_fun.R:
   - Grouped rows sharing identical fmt_fun definitions and resolved function aliases
     once per group rather than per row.
   - Vectorized formatting across statistic vectors for supported primitive and
     standard rounding functions, falling back to scalar evaluations only when required.
   - Maintained full condition capture parity (stat_fmt, warning, error) matching
     existing ARD S3 structure.

Verification and Impact:
- Unit tests: 865 passed, 0 failed, 0 warnings (test-ard_summary, test-ard_missing,
  test-nest_for_ard, test-apply_fmt_fun, test-ard_tabulate, etc.).
- cards::nest_for_ard() microbenchmark: 181 ms -> 28 ms (6.5x speedup, -87% memory allocation).
- .process_nested_list_as_df() microbenchmark: 4.71 ms -> 0.73 ms (6.5x speedup, -70% memory allocation).
- tbl_summary (50 vars): 797.6 ms -> 516.1 ms (1.55x speedup, -19% memory allocation).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant