Skip to content

Commit

Permalink
Link added to getOption 100x speedup submitted to R-core
Browse files Browse the repository at this point in the history
  • Loading branch information
@mattdowle
mattdowle committed Mar 17, 2018
1 parent b679215 commit 4814608
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion R/onLoad.R
View file
Expand Up @@ -31,7 +31,7 @@
# Set options for the speed boost in v1.8.0 by avoiding 'default' arg of getOption(,default=)
# In fread and fwrite we have moved back to using getOption's default argument since it is unlikely fread and fread will be called in a loop many times, plus they
# are relatively heavy functions where the overhead in getOption() would not be noticed. It's only really [.data.table where getOption default bit.
# TODO: submit improvement to .Internal(getOption(x)) in base::getOption to return NULL when option not set, to avoid (relatively slow) 'x %in% names(options())' there.
# Improvement to base::getOption() now submitted (100x; 5s down to 0.05s): https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17394

This comment has been minimized.

Sign in to view

Copy link
@MichaelChirico

MichaelChirico Mar 17, 2018 •
edited

Member

good. 👍 btw any reason not to submit %chin% to base?

This comment has been minimized.

Sign in to view

Copy link
@mattdowle

mattdowle Mar 17, 2018 •
edited

Author Member

In this getOption case simpler not to use %in% at all. But in general, reason is that %chin% uses the truelength-clobber trick to get its speed. The same trick is used in forder.c and that was accepted in base (a little to my surprise) in a localised way. As the years progress more people will discover that trick (like the Julia folks recently) and people in R-core. If no in-the-wild problems are reported (of R itself: ordering strings) then the trick could start to be used more widely, like by base::%in%. It would be a large patch. Good that %in% is used a lot in base and 10k packages as it would be well tested. %chin% was originally just for internal datatable use but as we got more confident in the trick (after years) the next step was exporting it. Yes next step could be base.

This comment has been minimized.

Sign in to view

Copy link
@MichaelChirico

MichaelChirico Mar 19, 2018

Member

Can't say I'm aware of the truelength-clobber trick? nothing in ?truelength
opts = c("datatable.verbose"="FALSE", # datatable.<argument name>
"datatable.nomatch"="NA_integer_", # datatable.<argument name>
"datatable.optimize"="Inf", # datatable.<argument name>
Expand Down

0 comments on commit 4814608

Please sign in to comment.