From ripley at stats.ox.ac.uk  Mon May 17 21:27:02 2004
From: ripley at stats.ox.ac.uk (Prof Brian Ripley)
Date: Mon May 17 21:27:09 2004
Subject: [Rd] request: allow inline functions in R
In-Reply-To: <35457.130.223.128.12.1084799788.squirrel@expasy-ng.isb-sib.ch>
Message-ID: <Pine.LNX.4.44.0405171423590.14150-100000@gannet.stats>

On Mon, 17 May 2004, Li Long wrote:

> > BTW, I've just checked a hunch. I did not believe R_IsNaNorNA is actually
> > used in base R on a system with IEEE arithmetic (and I removed it to test
> > that).  That suggests you have not actually tested inlining it and your
> > analysis is seriously flawed.
> 
> R_IsNaNorNA is found in three dirs: main/arithmatic.c, nmath/mlutils.c,
> nmath/nmath.h, include/R_ext/Arith.h and include/Rmath.h0.in.  (This is a
> candidate for streamlining the codes. But that's a different subject. )

However that part of nmath/mlutils.c is only used standalone and both it
and main/arithmetic.c only provided R_IsNaNorNA.  And in the current
sources where the non-IEEE case has been removed it is only in the first
two. Nothing calls R_IsNaNorNA. It is NOT used. You can remove it from all
those files in the current sources.  What is it you don't understand about
my second sentence?

Your final comment suggests that you do not understand how R's codebase is
organized.  A few parts are separated out (in a slightly different form)  
to make a standalone Rmath library *that does not use R's notion of NAs*.
The differences are larger than the commonality in the current version.  
We could easily streamline it by removing standalone nmath, but not
otherwise.

> After I turned all of them inline, the model I have runs about 5% faster

What is `them'?  Not `dirs', the grammatical immediate antecedent.

> on Itanium2 (running Linux).  When I cut down some calls in the package
> (being developed) to ISNAN, the performance gain is another 5%. I
> interprete this cost as quite significant.

I saw no measurable change in replacing all calls to R_finite by isfinite
(which is what we have done recently in 2.0.0 on platforms supporting
isfinite). That's at least as good as inlining, better on platforms on
which isfinite is a macro (as it is on two of those I tested).

BTW, 5% is the expected change in CPU performance in 1.25 months according
to Moore's Law (as stated on an Intel flyer).  It will be eclipsed by the 
speedup by the time R 2.0.0 is released.

> >> Could you then please quantify that hidden cost?
> 
> In package "cluster", pam.c and clara.c contain a function "ind_2" which
> is used very heavily in the loop. After changing this function to be an
> "inline" function, the model speeds up by factor of 2.  That's very
> significant improvement for the model.  It also indicates the overhead
> these small functions incur.

On your platform!  I did ask for examples on a commonly used platform, and 
for inlining in R, not a contributed package.

> >> In what sense do `we' not allow it?  And who is `we'?
> 
> What I meant for "we" is "R core developers".  At this time, R adhers
> strict C89 standard, so no inline function is allowed.

Well, I *am* an R core developer, and I didn't know that.  It is necessary
that R when configured on a strict C89 platform will compile and run,
which that is not what you claimed.  We do make use of several non-C89
features already on particular platforms or compilers.  We even include 
code using __inline__ (have you actually looked at the sources?).

> >> The problem is that very few compilers fully support C99, and others
> >> have
> >> different ways to indicate inlining.  So a configure test is needed. I
> >> am
> >> sure that if you provide one together with patches to parts of the code
> >> where you find inlining beneficial, the real `we' would consider it
> >> carefully.  Especially if the `hidden cost' is noticeable.

I am very interested in your response to this suggestion.

> I'm not aware of other ways of indicating "inlining" by other compilers,
> could you please give an example?

__CRT_INLINE by MinGW on Windows, for example.  __inline and __inline__
are also used.  For gcc __inline__ would be preferred to allow -pedantic
to be used (and that has helped catch a number of things which stricter 
compilers were rejecting).

> The request is to allowed "inline", not full C99 support.

`inline' is allowed *if* your package configure tests for it, so there is 
nothing to grant here.

You *will* find __inline__ used in the R sources, in the way it is most
effective, for static functions in a single compilation unit. The R
authors have tended to use macros instead, to the same effect.

> As indicated in my original request, compiler could choose to treat
> "inline" functions the same as normal functions. C++ supports "inline"
> functions from the beginning, and C99 puts it in as a new keyword, "gcc"
> supports it since 2.95, these point to a mature feature of the C language.

See above.  There are issues about inlining and functions imported from
DLLs and shared libraries, so we would need to ensure that the patches you
supply work with libR.so and R.dll.

> The inline functions are most suitable for small functions so that they
> don't stall the pipeline, and they should be used when they are most
> beneficial.  From this aspect, the portability and compatability issues
> are easily manageable.

Let me repeat the request for details of which functions, patches and
evidence of their effectiveness. You *still* have not told us exactly what
you believe to be effective: and I know R_IsNaNorNA (one of your few
examples) is not called in the current R sources.

-- 
Brian D. Ripley,                  ripley@stats.ox.ac.uk
Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
University of Oxford,             Tel:  +44 1865 272861 (self)
1 South Parks Road,                     +44 1865 272866 (PA)
Oxford OX1 3TG, UK                Fax:  +44 1865 272595


[Rd] request: allow inline functions in R
Prof Brian Ripley