Possible ALTREP bug

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Possible ALTREP bug

Gábor Csárdi
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

luke-tierney
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: Possible ALTREP bug

Jiefei Wang
In reply to this post by Gábor Csárdi
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Gábor Csárdi
In reply to this post by luke-tierney
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Jim Hester
In reply to this post by luke-tierney
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Gabriel Becker-2
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

luke-tierney
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Toby Hocking-2
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Oliver.P
*_ELT accessor functions are described in "vector accessor functions" in
Writing R extensions.

https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Vector-accessor-functions

On Wed, Jun 16, 2021 at 4:22 PM Toby Hocking <[hidden email]> wrote:

> By the way, where is the documentation for INTEGER_ELT, REAL_ELT, etc? I
> looked in Writing R Extensions and R Internals but I did not see any
> mention.
> REAL_ELT is briefly mentioned on
> https://svn.r-project.org/R/branches/ALTREP/ALTREP.html
> Would it be possible to please add some mention of them to Writing R
> Extensions?
> - how many of these _ELT functions are there? INTEGER, REAL, ... ?
> - in what version of R were they introduced?
> - I guess input types are always SEXP and int?
> - What are the output types for each?
>
> On Fri, May 28, 2021 at 5:16 PM <[hidden email]> wrote:
>
> > Since the INTEGER_ELT, REAL_ELT, etc, functions are fairly new it may
> > be possible to check that places where they are used allow for them to
> > allocate. I have fixed the one that got caught by Gabor's example, and
> > a rchk run might be able to pick up others if rchk knows these could
> > allocate. (I may also be forgetting other places where the _ELt
> > methods are used.)  Fixing all call sites for REAL, INTEGER, etc, was
> > never realistic so there GC has to be suspended during the method
> > call, and that is done in the dispatch mechanism.
> >
> > The bigger problem is jumps from inside things that existing code
> > assumes will not do that. Catching those jumps is possible but
> > expensive; doing anything sensible if one is caught is really not
> > possible.
> >
> > Best,
> >
> > luke
> >
> > On Fri, 28 May 2021, Gabriel Becker wrote:
> >
> > > Hi Jim et al,
> > > Just to hopefully add a bit to what Luke already answered, from what I
> am
> > > recalling looking back at that bioconductor thread Elt methods are used
> > in
> > > places where there are hard implicit assumptions that no garbage
> > collection
> > > will occur (ie they are called on things that aren't PROTECTed), and
> > beyond
> > > that, in places where there are hard assumptions that no error
> (longjmp)
> > > will occur. I could be wrong, but I don't know that suspending garbage
> > > collection would protect from the second one. Ie it is possible that an
> > > error *ever* being raised from R code that implements an elt method
> could
> > > cause all hell to break loose.
> > >
> > > Luke or Tomas Kalibera would know more.
> > >
> > > I was disappointed that implementing ALTREPs in R code was not in the
> > cards
> > > (it was in my original proposal back in 2016 to the DSC) but I trust
> Luke
> > > that there are important reasons we can't safely allow that.
> > >
> > > Best,
> > > ~G
> > >
> > > On Fri, May 28, 2021 at 8:31 AM Jim Hester <[hidden email]>
> > wrote:
> > >       From reading the discussion on the Bioconductor issue tracker it
> > >       seems like
> > >       the reason the GC is not suspended for the non-string ALTREP Elt
> > >       methods is
> > >       primarily due to performance concerns.
> > >
> > >       If this is the case perhaps an additional flag could be added to
> > >       the
> > >       `R_set_altrep_*()` functions so ALTREP authors could indicate if
> > >       GC should
> > >       be halted when that particular method is called for that
> > >       particular ALTREP
> > >       class.
> > >
> > >       This would avoid the performance hit (other than a boolean
> > >       check) for the
> > >       standard case when no allocations are expected, but allow
> > >       authors to
> > >       indicate that R should pause GC if needed for methods in their
> > >       class.
> > >
> > >       On Fri, May 28, 2021 at 9:42 AM <[hidden email]> wrote:
> > >
> > >       > integer and real Elt methods are not expected to allocate. You
> > >       would
> > >       > have to suspend GC to be able to do that. This currently can't
> > >       be done
> > >       > from package code.
> > >       >
> > >       > Best,
> > >       >
> > >       > luke
> > >       >
> > >       > On Fri, 28 May 2021, Gábor Csárdi wrote:
> > >       >
> > >       > > I have found some weird SEXP corruption behavior with
> > >       ALTREP, which
> > >       > > could be a bug. (Or I could be doing something wrong.)
> > >       > >
> > >       > > I have an integer ALTREP vector that calls back to R from
> > >       the Elt
> > >       > > method. When this vector is indexed in a lapply(), its first
> > >       element
> > >       > > gets corrupted. Sometimes it's just a type change to
> > >       logical, but
> > >       > > sometimes the corruption causes a crash.
> > >       > >
> > >       > > I saw this on macOS from R 3.5.3 to 4.2.0. I created a small
> > >       package
> > >       > > that demonstrates this:
> > >       https://github.com/gaborcsardi/redfish
> > >       > >
> > >       > > The R callback in this package calls
> > >       `loadNamespace("Matrix")`, but
> > >       > > the same crash happens for other packages as well, and
> > >       sometimes it
> > >       > > also happens if I don't load any packages at all. (But that
> > >       example
> > >       > > was much more complicated, so I went with the package
> > >       loading.)
> > >       > >
> > >       > > It is somewhat random, and sometimes turning off the JIT
> > >       avoids the
> > >       > > crash, but not always.
> > >       > >
> > >       > > Hopefully I am just doing something wrong in the ALTREP code
> > >       (see
> > >       > >
> > >       https://github.com/gaborcsardi/redfish/blob/main/src/test.c),
> > >       and it
> > >       > > is not actually a bug.
> > >       > >
> > >       > > Thanks,
> > >       > > Gabor
> > >       > >
> > >       > > ______________________________________________
> > >       > > [hidden email] mailing list
> > >       > > https://stat.ethz.ch/mailman/listinfo/r-devel
> > >       > >
> > >       >
> > >       > --
> > >       > Luke Tierney
> > >       > Ralph E. Wareham Professor of Mathematical Sciences
> > >       > University of Iowa                  Phone:
> > >        319-335-3386
> > >       > Department of Statistics and        Fax:
> > >        319-335-3017
> > >       >     Actuarial Science
> > >       > 241 Schaeffer Hall                  email:
> > >        [hidden email]
> > >       > Iowa City, IA 52242                 WWW:
> > >       http://www.stat.uiowa.edu
> > >       > ______________________________________________
> > >       > [hidden email] mailing list
> > >       > https://stat.ethz.ch/mailman/listinfo/r-devel
> > >       >
> > >
> > >               [[alternative HTML version deleted]]
> > >
> > >       ______________________________________________
> > >       [hidden email] mailing list
> > >       https://stat.ethz.ch/mailman/listinfo/r-devel
> > >
> > >
> > >
> >
> > --
> > Luke Tierney
> > Ralph E. Wareham Professor of Mathematical Sciences
> > University of Iowa                  Phone:             319-335-3386
> > Department of Statistics and        Fax:               319-335-3017
> >     Actuarial Science
> > 241 Schaeffer Hall                  email:   [hidden email]
> > Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu
> > ______________________________________________
> > [hidden email] mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
> >
>
>         [[alternative HTML version deleted]]
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Simon Urbanek
In reply to this post by Toby Hocking-2
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Toby Hocking-2
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

luke-tierney
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: [External] Possible ALTREP bug

Gabriel Becker-2
CONTENTS DELETED
The author has deleted this message.