Skip to content

Parallelize build_articles() and build_reference()#745

Closed
pat-s wants to merge 3 commits intor-lib:masterfrom
pat-s:articles-parallel
Closed

Parallelize build_articles() and build_reference()#745
pat-s wants to merge 3 commits intor-lib:masterfrom
pat-s:articles-parallel

Conversation

@pat-s
Copy link
Copy Markdown
Contributor

@pat-s pat-s commented Jun 24, 2018

#744

Uses future_map and future_walk from the furrr package if parallel = TRUE is set in build_site(), build_articles() or build_reference().

New arguments

  • parallel: Whether to run in parallel or not
  • progress: Whether to show a progress bar when running in parallel
  • workers: Number of cores used for parallelization. Default = all available cores / 2

Comments

  • By default all cores are used. It may be better to use only 50% by default as using all cores might conflict with vignettes that use parallelization themselves.

  • By default, the "Reading", "Writing" output is suppressed when building in parallel in favor of a progress bar. This is opinionated. What's your opinion on this? Another option would be to only suppress the "Reading", "Writing" output if argument progress = TRUE and to show it otherwise.

Example

build_site(parallel = TRUE, lazy = FALSE, workers = 2)
══ Building pkgdown site ══════════════════════════════════════════════════════════════════════════════════════════════════════════
Reading from: '/home/pjs/git/pkgdown'
Writing to:   '/home/pjs/git/pkgdown/docs'
── Initialising site ──────────────────────────────────────────────────────────────────────────────────────────────────────────────
Writing 'sitemap.xml'Edit '_pkgdown.yml'
── Building home ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Reading 'CODE_OF_CONDUCT.md'
Reading 'LICENSE.md'
Reading 'README.Rmd'
Writing 'index.html'
── Building function reference 
 Progress: ──────────────────────────────────100%

── Building articles 
 Progress: ──────────────────────────────────100%

── Building news ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Writing 'news/index.html'
══ DONE ═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
── Previewing site ────────────────────────────────────────────────────────────────────────────────────────────────────────────────

@hadley
Copy link
Copy Markdown
Member

hadley commented Jun 25, 2018

This kills the current progress reporting which makes errors difficult to debug. I think it requires more thought before we add parallel support to pkgdown.

@pat-s
Copy link
Copy Markdown
Contributor Author

pat-s commented Jun 25, 2018

This kills the current progress reporting which makes errors difficult to debug. I think it requires more thought before we add parallel support to pkgdown.

I turned it off on purpose. If you prefer to keep the progress reporting using the "Reading/writing" approach as it applies for the sequential execution, I'll change it again.

What other potential problems do you have in mind that I am not aware off?

@pat-s pat-s force-pushed the articles-parallel branch from 6ee6d97 to 9201e3d Compare June 29, 2018 21:24
@pat-s
Copy link
Copy Markdown
Contributor Author

pat-s commented Jun 29, 2018

I would bet that I saw "Reading/Writing" output when I first integrated future_walk(). This surprised me at first. Well, probably I was wrong and just saw the sequential run in this moment...

I changed the code to use half of all cores so that it does not conflict with vignettes having parallel code.

I would not see the requirement for debugging; the sequential mode can be used for this. Once one is sure that everything works, the parallel mode can be used.

If this PR is most likely not going to be merged/useful right now, feel free to close it.

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.

3 participants