Skip to content

Support user-settable memory allocation limit#346

Merged
eddelbuettel merged 4 commits intomasterfrom
de/sc-13173/var_alloc_defaults
Jan 14, 2022
Merged

Support user-settable memory allocation limit#346
eddelbuettel merged 4 commits intomasterfrom
de/sc-13173/var_alloc_defaults

Conversation

@eddelbuettel
Copy link
Copy Markdown
Contributor

Memory requirements for sparse arrays are difficult to asses ex-ante. This PR adds a user-setable parameter which will be used for variable-sized character buffers.

The setup follows what is used for the return_as preference. A value can stored in per-user config file (with location managed by R using its tools::R_user_dir() function following XDG conventions) from where it is retrieve and used in the per-package run-time environment. The value can also be override per-session.

@eddelbuettel eddelbuettel requested a review from ihnorton January 10, 2022 19:48
@shortcut-integration
Copy link
Copy Markdown

This pull request has been linked to Shortcut Story #13173: Be smarter about minimal size allocated for character data.

@eddelbuettel eddelbuettel force-pushed the de/sc-13173/var_alloc_defaults branch 2 times, most recently from f616b8d to 8b7eaa0 Compare January 11, 2022 16:05
Copy link
Copy Markdown
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, let's change this to be per-column.

@eddelbuettel
Copy link
Copy Markdown
Contributor Author

Now I recall t he other reason why I divided: the value from our config object is somewhat yuge. Using it per column may lead to multiple yuge allocations. On the other hand, it affects (var)char only, and I pull the non-char value.

@eddelbuettel eddelbuettel force-pushed the de/sc-13173/var_alloc_defaults branch from 8b7eaa0 to fc3e3e1 Compare January 13, 2022 21:52
@eddelbuettel eddelbuettel force-pushed the de/sc-13173/var_alloc_defaults branch from fc3e3e1 to 3cd454f Compare January 13, 2022 21:53
@ihnorton ihnorton changed the title Support user-setable memory allocation limit Support user-settable memory allocation limit Jan 14, 2022
Copy link
Copy Markdown
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@eddelbuettel
Copy link
Copy Markdown
Contributor Author

As discussed, with one more pass over it for a default value which makes the initial setting simpler -- load* called at package load, lightweight getters afterwards. (One minor documentation warning fixed by tiny doc edit too.)

@ihnorton
Copy link
Copy Markdown
Member

Thanks!

@eddelbuettel eddelbuettel force-pushed the de/sc-13173/var_alloc_defaults branch from 249cc48 to 5345d72 Compare January 14, 2022 20:25
@eddelbuettel eddelbuettel merged commit 9068acc into master Jan 14, 2022
@eddelbuettel eddelbuettel deleted the de/sc-13173/var_alloc_defaults branch January 14, 2022 20:40
@eddelbuettel eddelbuettel mentioned this pull request Jan 24, 2022
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.

2 participants