remove allocNAVector in melt for memory efficiency#5054
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While hacking various improvements to melt recently, I noticed that there were un-necessary allocations of temporary vectors of missing values via allocNAVector, when there are missing input columns. In this PR I propose to make melt more memory efficient by replacing those temporary NA vectors with NULL/R_NilValue which serves as a signal to either (1) do nothing if na.rm=TRUE, or (2) use writeNA on the output vector if na.rm=FALSE. There are no changes to user-facing functionality, but hopefully melt should be more efficient when there are missing input columns.