Skip to content

Improve speed using stringi package#440

Open
noamross wants to merge 5 commits intoawalker89:masterfrom
noamross:master
Open

Improve speed using stringi package#440
noamross wants to merge 5 commits intoawalker89:masterfrom
noamross:master

Conversation

@noamross
Copy link

This is an incomplete PR for feedback before I continue with it. It replaces a number of calls to paste0() and paste() with stringi::stri_join(), which is approximately 4x faster.

This came up in a project of mine where I needed to write a create a large number of styles and apply them to a large number of cells (in many workbooks!). We found a speed bottleneck in the paste() functions at https://github.com/awalker89/openxlsx/blob/master/R/WorkbookClass.R#L2105-L2106 , so these changes helped speed up the process and my team is using this fork.

If you are interested in this PR, I'd be happy to go through the rest of the package and replace calls (paste(), gsub(), grepl(), date formatting, etc.) that could be accelerated with their stringi equivalents, which in my experience are 2-4x faster.

stringi is a mixed bag as a dependency - it takes a while to install from source because it contains all of unicode, but it has no non-base dependencies and many people have it already as it is a tidyverse dependency. In my experience it is also quite stable.

@GegznaV
Copy link

GegznaV commented Oct 7, 2019

I think speed gives value. @noamross, Can this pull request be directed to the new home of openxlsx: https://github.com/ycphs/openxlsx/ (see ycphs/openxlsx#2)

@ycphs
Copy link
Contributor

ycphs commented Oct 8, 2019

Unfortunately the the checks fail. @noamross: Please could you check once more the. I found a problem in the saveWorkbook method.

The examples and the tests are showing the same failure.

ycphs added a commit to ycphs/openxlsx that referenced this pull request Nov 11, 2019
compare to Improve speed using stringi package #440 (awalker89/openxlsx#440)
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