Speed up is_installed() - shortcut when package is already loaded

[eitsupi]

Same as #1780

[shikokuchuo]

Thanks @eitsupi looks good to me!

[MichaelChirico]

Is this a typical intended interface? is_installed(list("dplyr", "dbplyr"))

If not, we can move the checks "outside the loop" toget early exits for any of the !is.character(pkg) and !all(nzchar(pkg)) cases.

Next, why use is.environment(.getNamespace(x)) instead of isNamespaceLoaded(x)? The former is described as an "internal" (hence the leading .), the latter is more readable (and nanosecond-scale-faster).

Suggested change

	if (is.null(version)) {
	loaded <- lapply(pkg, function(x) {
	is.character(x) && nzchar(x) && is.environment(.getNamespace(x))
	})
	if (all(as.logical(loaded))) {
	return(TRUE)
	}
	}
	if (is.null(version) && is.character(pkg) && all(nzchar(pkg)) && all(vapply(pkg, isNamespaceLoaded, NA))) {
	return(TRUE)
	}

[shikokuchuo]

The argument is expected to be a character vector c("dplyr", "dbplyr") rather than a list, so your suggestion would work well in that case. A list would just fail the fast path but continue, so is not disastrous.

If we make the change here, we should do the same at check_installed().

Thanks for the suggestion of isNamespaceLoaded(x). It is indeed faster as it shares the same C level code path as .getNamespace(x) but just returns a logical value rather than the environment itself.

[shikokuchuo]

@MichaelChirico sorry somehow I missed your entire code suggestion, so my inital reply was totally off the mark (now amended above).

[eitsupi]

Thanks for the suggestions!

Would it be better to refactor this with check_installed after merging this since my intention of this PR is just to bring the same changes here as check_installed in #1780?

[shikokuchuo]

Yes that's fine thanks! If Lionel adopts the suggestion here, I will follow up with the corresponding change to check_installed().