From haverty.peter at gene.com  Fri Jan 23 18:13:57 2015
From: haverty.peter at gene.com (Peter Haverty)
Date: Fri, 23 Jan 2015 09:13:57 -0800
Subject: [Rd] speedbump in library
In-Reply-To: <CAFOpNVFT3hXcfbDk2fPV2ZyH=NonfZ-Nys05O4E1qWH_kJ8XAA@mail.gmail.com>
References: <CAGh0NYoAgB-ydT2j0eynAcRMefmL6C3m81dD-a6ZfDmN3kO+tw@mail.gmail.com>
	<CAFOpNVFT3hXcfbDk2fPV2ZyH=NonfZ-Nys05O4E1qWH_kJ8XAA@mail.gmail.com>
Message-ID: <CAGh0NYobWb6CkEf_8kjR5W5U52uWUccjead0M8-+E0HJTaq_gw@mail.gmail.com>

Thanks Winston,

Yes, your version of that part is more direct. I guess it would need a
! is.null() too. I think we should use .getNamespace.

It It also occurred to me that this %in% check (which happens in a few
places) is kind of roundabout. It equates to

"foo" %in% ls(.Internal(getNamespaceRegistry()), all.names = TRUE)

We lack and R-level accessor for the namespace registry, but if we had
one we could do

getNamespaceRegistry()[["foo"]]

, which is just a hash lookup.



I'm getting a bit off topic here, but ...

"foo" %in% vector is a common pattern and reads well, but

any(vector == "foo")

is less work and much faster.  I wonder if there is room for a fast
path there ...



Pete

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


On Fri, Jan 23, 2015 at 8:15 AM, Winston Chang <winstonchang1 at gmail.com> wrote:
> I think you can simplify a little by replacing this:
>   pkg %in% loadedNamespaces()
> with this:
>   .getNamespace(pkg)
>
> Whereas getNamespace(pkg) will load the package if it's not already
> loaded, calling .getNamespace(pkg) (note the leading dot) won't load
> the package.
>
> I can't speak to whether there are any pitfalls in changing the
> library path searching, though.
>
> -Winston
>
>
> On Thu, Jan 22, 2015 at 12:25 PM, Peter Haverty <haverty.peter at gene.com> wrote:
>> Hi all,
>>
>> Profiling turned up a bit of a speedbump in the library function. I
>> submitted a patch to the R bug tracker as bug 16168 and I've also
>> included it below. The alternate code is simpler and easier to
>> read/maintain, I believe.  Any thoughts on other ways to write this?
>>
>> Index: src/library/base/R/library.R
>> ===================================================================
>> --- src/library/base/R/library.R    (revision 67578)
>> +++ src/library/base/R/library.R    (working copy)
>> @@ -688,18 +688,8 @@
>>      out <- character()
>>
>>      for(pkg in package) {
>> -        paths <- character()
>> -        for(lib in lib.loc) {
>> -            dirs <- list.files(lib,
>> -                               pattern = paste0("^", pkg, "$"),
>> -                               full.names = TRUE)
>> -            ## Note that we cannot use tools::file_test() here, as
>> -            ## cyclic namespace dependencies are not supported.  Argh.
>> -            paths <- c(paths,
>> -                       dirs[dir.exists(dirs) &
>> -                            file.exists(file.path(dirs,
>> -                                                  "DESCRIPTION"))])
>> -        }
>> +        paths <- file.path(lib.loc, pkg)
>> +        paths <- paths[ file.exists(file.path(paths, "DESCRIPTION")) ]
>>          if(use_loaded && pkg %in% loadedNamespaces()) {
>>              dir <- if (pkg == "base") system.file()
>>              else getNamespaceInfo(pkg, "path")
>>
>> Pete
>>
>> ____________________
>> Peter M. Haverty, Ph.D.
>> Genentech, Inc.
>> phaverty at gene.com
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel



[Rd] speedbump in library
Peter Haverty