remove all bottlenecks in creating instrList with multiple sortIDs#44
Open
paciorek wants to merge 4 commits into
Open
remove all bottlenecks in creating instrList with multiple sortIDs#44paciorek wants to merge 4 commits into
paciorek wants to merge 4 commits into
Conversation
lists of instr_nClass-like R lists.
Contributor
Author
|
Also we'll want to discuss whether anything in the setup code processing in nimble2 needs adjustment given that |
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
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.
This gives huge speedups when we have multiple sortID values in a calcRange and split into individual instructions. Formerly we were creating multiple calcRanges and then multiple instr_nClass objects.
This avoids initializing all those R6 class objects. Instead the
calcRangemethods splits the range into a list of multiple R lists where each R list is essentially the same form as an instr_nClass (i.e., the same named fields).Before it took 40 ms to make an instr_nClass object and 0.5 ms to make a calcRangeClass object. The time to make a simple R list is negligible by comparison (I didn't microbenchmark that). The result is that for 10000 elements (y[i] <- y[i+1]), it used to take 500 seconds to make the instruction list and now takes 0.03. So a million such elements now takes 3 seconds.
For handling the resulting R list of "R instr", for the moment I made it an S3 class "Rlist_Rinstr". We could consider making an R6 class for the overall list and an R6 class for the inner lists. At the least if we do the former, we can give it an
isCompiledmethod and that will fit into the existing checking incalc_opandsimulate. Using S3 doesn't fit within our standard style.This feels a bit like working around the system we devised but the working around is primarily within
makeInstrList, and the speedup is huge, so seems ok.