[Rd] R_check_class_etc(x, valid) is "slow" when 'valid' contains class(x)
Mikael Jagan
j@g@nmn2 @end|ng |rom gm@||@com
Fri Sep 2 14:51:17 CEST 2022
On 2022-09-02 5:30 am, Tomas Kalibera wrote:
> Hi Mikael,
>
> On 8/28/22 01:13, Mikael Jagan wrote:
>> R_check_class_etc(x, valid) spends a nontrivial amount of time finding
>> an environment 'rho' containing the definition of class(x), evaluating
>> (in R, not C) methods::.classEnv(class(x)).
>>
>> It then returns the result of R_check_class_and_super(x, valid, rho).
>> But R_check_class_and_super() does not use 'rho' at all in the trivial
>> case where class(x) is found in 'valid'.
> right, that could be improved. Do you have an example which exhibits the
> problem, have you found this by profiling something?
I benchmarked .Call("R_etc", x) for x <- new("dgTMatrix"), after compiling
and loading the following:
```
#include <Rinternals.h>
SEXP R_etc(SEXP x) {
static const char *valid[] = {"dgCMatrix", "dgRMatrix", "dgTMatrix", ""};
return ScalarInteger(R_check_class_etc(x, valid));
}
```
R-devel used ~2 microseconds while my patched version of R used ~0.2
microseconds. So, not a bottleneck by any means. On the other hand,
'Matrix' and probably other packages do call R_check_class_etc() quite
often, hence my report.
>>
>> My feeling is that this can be improved. I am happy to contribute a patch,
>> if it would be considered by R-core.
>
> Both R_check_class_etc and R_check_class_and_super are unfortunately exported,
> the former is used a lot in packages (even though they are not mentioned in
> Writing R Extensions, so actually shouldn't be used in packages). Anyway, it
> would be easier if we could preserve their interface and behavior.
>
> Maybe we could support rho==NULL in R_check_class_and_super, the environment
> would be looked up in that case when needed. R_check_class_etc would simply only
> call R_check_class_and_super with that argument. I see that
> R_check_class_and_super uses asChar() on the class attribute, while
> R_check_class_etc does not currently for looking for the environment, but I
> assume doing that in both cases should not matter (and it would have to be tested).
>
> So this would be a trivial change, but if you wanted to create a minimal patch,
> I will be happy to have a look.
I've implemented essentially what you've described; see attached.
Mikael
>
> Best
> Tomas
>
>
>> Mikael
>>
>> ______________________________________________
>> R-devel using r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.diff
URL: <https://stat.ethz.ch/pipermail/r-devel/attachments/20220902/7e17c4be/attachment.ksh>
More information about the R-devel
mailing list