Skip to content

:: and ::: as .Primitives?

13 messages · Peter Haverty, Luke Tierney, Tim Keitt +6 more

Original
Peter Haverty
# 
Hi all,

When S4 methods are defined on base function (say, "match"), the
function becomes a method with the body "base::match(x,y)". A call to
such a function often spends more time doing "::" than in the function
itself.  I always assumed that "::" was a very low-level thing, but it
turns out to be a plain old function defined in base/R/namespace.R.
What would you all think about making "::" and ":::" .Primitives?  I
have submitted some examples, timings, and a patch to the R bug
tracker (https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=16134).
I'd be very interested to hear your thoughts on the matter.

Regards,
Pete

____________________
Peter M. Haverty, Ph.D.
Genentech, Inc.
phaverty at gene.com
Luke Tierney
# 
I'm not convinced that how to make :: faster is the right question. If
you are finding foo::bar being called often enough to matter to your
overall performance then to me the question is: why are you calling
foo::bar more than once? Making :: a bit faster by making it a
primitive will remove some overhead, but your are still left with a
lot of work that shouldn't need to happen more than once.

For default methods there ought to be a way to create those so the
default method is computed at creation or load time and stored in an
environment. For other cases if I want to use foo::bar many times, say
in a loop, I would do

foo_bar <- foo::bar

and use foo_bar, or something along those lines.

When :: and ::: were introduce they were intended primarily for
reflection and debugging, so speed was not an issue. ::: is still
really only reliably usable that way, and making it faster may just
encourage bad practice. :: is different and there are good arguments
for using it in code, but I'm not yet seeing good arguments for use in
ways that would be performance-critical, but I'm happy to be convinced
otherwise. If there is a need for a faster :: then going to a
SPECIALSXP is fine; it would also be good to make the byte code
compiler aware of it, and possibly to work on ways to improve the
performance further e.g. through cacheing.

Best,

luke
On Thu, 22 Jan 2015, Peter Haverty wrote:

            

      
      
      
    

        

      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Tim Keitt
    

    

      
      #
    

  


  

      
On Thu, Jan 22, 2015 at 1:44 PM, <luke-tierney at uiowa.edu> wrote:

        

            

      
      
      
    

        
I think you will find that no matter how much it does not matter in terms
of performance, folks will avoid :: out of principle if they think its
slower. We're conditioned to write efficient code even when it does not
really impact real world usage. As using :: is good practice in many
contexts, making it fast will encourage folks to use it.

THK

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Henrik Bengtsson
    

    

      
      #
    

  


  

      
On Thu, Jan 22, 2015 at 11:44 AM,  <luke-tierney at uiowa.edu> wrote:

            

      
      
      
    

        
While you're on the line: Do you think this is an optimization that
the 'compiler' package and it's cmpfun() byte compiler will be able to
do in the future?

/Henrik

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Michael Lawrence
    

    

      
      #
    

  


  

      
On Thu, Jan 22, 2015 at 11:44 AM,  <luke-tierney at uiowa.edu> wrote:

            

      
      
      
    

        
We had considered that, but we thought the definition of the function
would be easier to interpret if it explicitly specified the namespace,
instead of using tricks with environments. The same applies for
memoizing the lookup in front of a loop.

The implementation of these functions is almost simpler in C than it
is in R, so there is relatively little risk to this change. But I
agree the benefits are also somewhat minor.

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Peter Haverty
    

    

      
      #
    

  


  

      
Hi all,

I use Luke's "::" hoisting trick often. I think it would be fantastic
if the JIT just did that for you.

The main trouble, for me, is in code I don't own.  When common
Bioconductor packages are loaded many, many base functions are saddled
with this substantial dispatch and "::" overhead.

While we have the hood up, the parser could help out a bit here too.
It already has special cases for "::" and ":::". Currently you get the
symbols "pkg" and "name" and have to go fishing in the calling
environment for the associated values.  It would be nice to have the
parser or JIT rewrite base::match as doubleColon("base","match") or
directly provide the symbols "base" and "match" to the subsequent
code.

I think it's also kind of entertaining that the comments in
base/R/namespace.R note that they are using ":::" for speed purposes
only.
Pete

____________________
Peter M. Haverty, Ph.D.
Genentech, Inc.
phaverty at gene.com


On Thu, Jan 22, 2015 at 12:54 PM, Michael Lawrence

        
<lawrence.michael at gene.com> wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      William Dunlap
    

    

      
      #
    

  


  

          

      
      
      
    

        
The foreach package does that with a function from the compiler package,
so that foreach can work on old version of R:
  comp <- if (getRversion() < "2.13.0") {
    function(expr, ...) expr
  } else {
    compiler::compile
  }
This results in foreach having its own copy of compiler::compile, with
namespace "compiler", but copied from the version of package:compile
existing on the machine that built the binary of foreach.  If you later
install
an updated version of the compiler package, then foreach still uses the old
compiler::compile, which may not work with the private functions in
the new version of package:compiler.

Making :: faster would not fix this particular problem (making 'comp' a
function that contained the if(getRVersion...) code would), but things
like this could cause problems when more people put 'myFunc <-
otherPackage::Func'
in their packages.




Bill Dunlap
TIBCO Software
wdunlap tibco.com

        
On Thu, Jan 22, 2015 at 11:44 AM, <luke-tierney at uiowa.edu> wrote:

        

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
  


  


      

    

    

      
      

        


  

        

      Luke Tierney
    

    

      
      #
    

  


  

      
On Thu, 22 Jan 2015, Henrik Bengtsson wrote:

        

            

      
      
      
    

        
Most likely, at least at reasonable optimization levels.

Best,

luke

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Luke Tierney
    

    

      
      #
    

  


  

      
On Thu, 22 Jan 2015, Michael Lawrence wrote:

        

            

      
      
      
    

        
interpret in what sense (human reader or R interpreter)? In either
case I'm not convinced.

            

      
      
      
    

        
I don't disagree, but it remains that even calling the C version has
costs that should not need to be paid. But maybe we can leave that to
the compiler/byte code engine. Optimizing references to symbols
resolved statically to name spaces and imports is on the to do list,
and with a little care that mechanism should work for foo::bar uses as
well.

Best,

luke

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Philippe GROSJEAN
    

    

      
      #
    

  


  

      
I tend to use this (in my own internal code *only*):

exported <- function (pkg) {
	if (pkg == "base") {
		function (fun) {
			fun <- as.character(substitute(fun))
			res <- .BaseNamespaceEnv[[fun]]
			if (is.null(res))
				stop(fun, " is not found in package base")
			res
		}
	} else {
		ns <- getNamespace(pkg)
		exports <- getNamespaceInfo(ns, "exports")
		function (obj) {
			obj <- as.character(substitute(obj))
			exportedObj <- exports[[obj]]
			if (is.null(exportedObj)) {
				if (is.null(ns[[obj]])) {
					stop(obj, " does not exists in package ", pkg)	
				} else {
					stop(obj, " is not exported from package ", pkg)
				}
			}
			ns[[exportedObj]]
		}
	}
}
stats <- exported("stats")
stats(acf)
stats("[.acf")
stats("inexistant")
exported("base")(ls)
exported("base")(inexistant)

## Performance tests for what it?s worth
microbenchmark::microbenchmark(stats::acf, (stats <- exported("stats"))(acf), stats(acf))
microbenchmark::microbenchmark(base::ls, (base <- exported("base"))(ls), base(ls), .BaseNamespaceEnv$ls)

So, `::` is slow and I can get better speed results thanks to binding both the namespace and the exports environments in the `stats` closure. Unless I miss something, this is not much a problem for base package that is never unloaded. Yet, .BaseNamespaceEnv$xxx, or baseenv()$xxx does the job faster and simpler. 

However, there is a vicious problem with my exported() function, which is, to say the least, dangerous under the hand of unaware users. Indeed:

stats <- exported(?stats?)

creates a new binding to both the namespace and the exports environments of the stats package. So, if I do:

detach(?package:stats?, unload = TRUE), then library(?stats?), I got two versions of the package in memory, and my `stats`closure refers to an outdated version of the package. This is particularly problematic if the package was recompiled in between (in the context of debugging).

Conclusion: much of the lost of performance in `::` is due to not caching the environments. This is fully justified to keep the dynamism of the language at full power and to avoid a messy state of R as described here above? Regarding dynamism, even `stats::acf`remains discutable.

Moreover, it is possible to do many other crazy things with these environments, once one got a grip on them. So, even getNamespace() and getNamespaceInfo() are dangerous. Perhaps this should be emphasised in the ?getNamespace man page?

This is also why the code above is not released in the wild? Well, now it is :-(

Best,

Philippe

..............................................<?}))><........
 ) ) ) ) )
( ( ( ( (    Prof. Philippe Grosjean
 ) ) ) ) )
( ( ( ( (    Numerical Ecology of Aquatic Systems
 ) ) ) ) )   Mons University, Belgium
( ( ( ( (
..............................................................

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Hervé Pagès
    

    

      
      #
    

  


  

      
Hi,

        
On 01/23/2015 07:01 AM, luke-tierney at uiowa.edu wrote:

            

      
      
      
    

        
From a developer perspective, especially when debugging, when we do
selectMethod("match", ...) and it turns out that this returns the
default method, it's good to see:

   Method Definition (Class "derivedDefaultMethod"):

   function (x, table, nomatch = NA_integer_, incomparables = NULL,
       ...)
   base::match(x, table, nomatch = nomatch, incomparables = incomparables,
       ...)
   <environment: namespace:BiocGenerics>

   Signatures:
           x           table
   target  "DataFrame" "ANY"
   defined "ANY"       "ANY"

rather than some obscure/uninformative body. I hope we can keep that.

            

      
      
      
    

        
That would be great. Thanks!

H.

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Duncan Murdoch
    

    

      
      #
    

  


  

      
On 22/01/2015 4:06 PM, Peter Haverty wrote:

            

      
      
      
    

        
I don't think the parser should do this, but it does seem like a
reasonable optimization for the compiler to do.

It would be nice to have the

            

      
      
      
    

        
Currently the parser provides the expression `::`(base, match), and the
`::` function converts those symbols to character strings "base" and
"match".  While the parser could have saved it some work by giving the
expression `::`("base", "match"), I think it's a bad idea to start
messing with things that way.  After all, a user could have defined
their own `::` function, and they should get what they typed.

Duncan Murdoch

  
  


  


      

    

    

      
      

        


  

        

      Michael Lawrence
    

    

      
      #
    

  


  

      
On Fri, Jan 23, 2015 at 10:11 AM, Herv? Pag?s <hpages at fredhutch.org> wrote:

            

      
      
      
    

        
That was the goal of this patch. We want to keep that, and make
match() ~25% faster when falling back to the default method (for small
inputs). Right now, loading BiocGenerics, IRanges, etc, slows many
functions down by roughly that amount.