Issue #8399 has been updated by naruse (Yui NARUSE).


dbussink (Dirkjan Bussink) wrote:
> naruse (Yui NARUSE) wrote:
> > ko1's rgengc plans to introduce RARRAY_AREF for this purpose (see #8339 for detail).
> > So use it.
> 
> In Rubinius we also still support 1.8 and 1.9 modes and use the extensions from those modes. I understand 1.8 not being updated, but it would be very useful for us to also be able to update the 1.9 extensions with this. Will RARRAY_AREF be backported? And how about storing an entry? Is there also a macro for storing an entry?
> 
> What is the general recommendation for extension writers? Use RARRAY_AREF or rb_ary_entry()? I think the extensions MRI ships should be an example of how the Ruby community things extensions should be written, so they should be in a style others should be comfortable with copying.

dbussink (Dirkjan Bussink) wrote:
> naruse (Yui NARUSE) wrote:
> > ko1's rgengc plans to introduce RARRAY_AREF for this purpose (see #8339 for detail).
> > So use it.
> 
> In Rubinius we also still support 1.8 and 1.9 modes and use the extensions from those modes. I understand 1.8 not being updated, but it would be very useful for us to also be able to update the 1.9 extensions with this. Will RARRAY_AREF be backported? And how about storing an entry? Is there also a macro for storing an entry?
> 
> What is the general recommendation for extension writers? Use RARRAY_AREF or rb_ary_entry()? I think the extensions MRI ships should be an example of how the Ruby community things extensions should be written, so they should be in a style others should be comfortable with copying.

As eregon says ko1 added RARRAY_AREF in r40689.

Anyway you can define compatible layer like

#ifndef RARRAY_AREF
# define RARRAY_AREF(a, i)    (RARRAY_PTR(a)[i])
#endif
#ifndef RARRAY_ASET
# define RARRAY_ASET(a, i, v) do {RARRAY_PTR(a)[i] = (v);} while (0)
#endif

or

#ifndef RARRAY_AREF
# define RARRAY_AREF(a, i)    rb_ary_entry((a), (i))
#endif
#ifndef RARRAY_ASET
# define RARRAY_ASET(a, i, v) rb_ary_store((a), (i, (v)))
#endif
----------------------------------------
Bug #8399: Remove usage of RARRAY_PTR in C extensions when not needed
https://bugs.ruby-lang.org/issues/8399#change-39303

Author: dbussink (Dirkjan Bussink)
Status: Open
Priority: Normal
Assignee: 
Category: 
Target version: 
ruby -v: trunk
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


Rubinius uses quite a few C extensions directly from MRI. Some of these use functionality such as RARRAY_PTR which is not necessary. For compatibility reasons, RARRAY_PTR works on Rubinius but suffers from a heavy performance penalty. Take for example the test of the parser gem (http://github.com/whitequark/parser). These run over 10x faster with the patch applied to Racc that is submitted here:

https://gist.github.com/dbussink/57c32c08fb21c7a41719

Consider issue #8339 where there is work being done on generational GC, I think it is also beneficial to remove usage of internal structures such as RARRAY_PTR where there is the problem of going around the write barrier. In Rubinius, an array is treated special if RARRAY_PTR is used on it in the C-API, so I can imagine MRI being able to optimize the GC better if extensions don't do this. There are functions available for both getting and setting elements in an array and they work fine.

I have only make a patch against Racc here as a showcase, I also want to update all the other extensions to remove RARRAY_PTR. Please consider this change to MRI since in my opinion it has benefits also for MRI and so Rubinius can keep using these extensions directly without having to maintain custom versions just for the considerations described here. I'm also already actively checking C extension gems and sending pull requests for updating this.



-- 
http://bugs.ruby-lang.org/