Skip to content

Commit a50c8e5

Browse files
eregoneregon
committed
[GR-18163] Unwrap VALUE* args for rb_funcallv* and rb_funcall_with_block directly
PullRequest: truffleruby/2706
2 parents 84d5d61 + 1c7d9b8 commit a50c8e5

File tree

12 files changed

+149
-65
lines changed

12 files changed

+149
-65
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ Performance:
4747
* `TruffleSafepoint` is now used instead of custom logic, which no longer invalidates JITed code for guest safepoints (e.g., `Thread#{backtrace,raise,kill}`, `ObjectSpace`, etc)
4848
* Significantly improved performance of `Time#strftime` for common formats (#2361, @wildmaples, @chrisseaton).
4949
* Faster solution for lazy integer length (#2365, @lemire, @chrisseaton).
50+
* Speedup `rb_funcallv*()` by directly unwrapping the C arguments array instead of going through a Ruby `Array` (#2089).
5051

5152
Changes:
5253

‎lib/cext/ABI_check.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1
1+
2

‎lib/cext/include/truffleruby/truffleruby-pre.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ extern "C" {
4646
typedef unsigned long VALUE;
4747
typedef unsigned long ID;
4848

49+
POLYGLOT_DECLARE_TYPE(VALUE)
50+
4951
// Support
5052

5153
extern void* rb_tr_cext;

‎lib/truffle/truffle/cext.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -865,16 +865,16 @@ def rb_cmpint(val, a, b)
865865
end
866866
end
867867

868-
def rb_funcall_with_block(recv, meth, args, block)
869-
Primitive.public_send_without_cext_lock(recv, meth, args, block)
868+
def rb_funcall_with_block(recv, meth, argv, block)
869+
Primitive.public_send_argv_without_cext_lock(recv, meth, argv, block)
870870
end
871871

872-
def rb_funcallv_public(recv, meth, args)
873-
Primitive.public_send_without_cext_lock(recv, meth, args, nil)
872+
def rb_funcallv_public(recv, meth, argv)
873+
Primitive.public_send_argv_without_cext_lock(recv, meth, argv, nil)
874874
end
875875

876-
def rb_funcallv(recv, meth, args)
877-
rb_funcall(recv, meth, nil, *args)
876+
def rb_funcallv(recv, meth, argv)
877+
Primitive.send_argv_without_cext_lock(recv, meth, argv, nil)
878878
end
879879

880880
def rb_funcall(recv, meth, n, *args)

‎lib/truffle/truffle/cext_ruby.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ module Truffle::CExt
1515
# methods defined with rb_define_method are normal Ruby methods therefore they cannot be defined in the cext.rb file
1616
# file because blocks passed as arguments would be skipped by org.truffleruby.cext.CExtNodes.BlockProcNode
1717
def rb_define_method(mod, name, function, argc)
18-
if argc < -2
19-
raise "Unsupported rb_define_method argc: #{argc}"
18+
if argc < -2 or 15 < argc
19+
raise ArgumentError, "arity out of range: #{argc} for -2..15"
2020
end
2121

2222
method_body = Truffle::Graal.copy_captured_locals -> *args, &block do

‎src/main/c/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ clean_truffleposix:
4242

4343
clean_cexts:
4444
$(Q) rm -f cext/Makefile cext/*.o $(LIBTRUFFLERUBY)
45-
$(Q) rm -f openssl/Makefile openssl/*.o openssl/openssl.$(DLEXT)
45+
$(Q) rm -f openssl/Makefile openssl/*.o openssl/openssl.$(DLEXT) openssl/extconf.h
4646
$(Q) rm -f zlib/Makefile zlib/*.o zlib/zlib.$(DLEXT)
4747
$(Q) rm -f psych/Makefile psych/*.o psych/yaml/*.o psych/psych.$(DLEXT)
4848
$(Q) rm -f ripper/Makefile ripper/*.o ripper/ripper.$(DLEXT)

‎src/main/c/cext/array.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,8 @@ VALUE rb_ary_new_from_args(long n, ...) {
5151
}
5252

5353
VALUE rb_ary_new_from_values(long n, const VALUE *values) {
54-
VALUE array = rb_ary_new_capa(n);
55-
for (int i = 0; i < n; i++) {
56-
rb_ary_store(array, i, values[i]);
57-
}
58-
return array;
54+
return rb_tr_wrap(polyglot_invoke(RUBY_CEXT, "rb_ary_new_from_values",
55+
polyglot_from_VALUE_array(values, n)));
5956
}
6057

6158
VALUE rb_assoc_new(VALUE a, VALUE b) {

‎src/main/c/cext/call.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ int rb_respond_to(VALUE object, ID name) {
2424
}
2525

2626
VALUE rb_funcallv(VALUE object, ID name, int args_count, const VALUE *args) {
27-
return RUBY_CEXT_INVOKE("rb_funcallv", object, ID2SYM(name), rb_ary_new4(args_count, args));
27+
return rb_tr_wrap(polyglot_invoke(RUBY_CEXT, "rb_funcallv",
28+
rb_tr_unwrap(object),
29+
rb_tr_unwrap(ID2SYM(name)),
30+
polyglot_from_VALUE_array(args, args_count)));
2831
}
2932

3033
VALUE rb_funcallv_kw(VALUE object, ID name, int args_count, const VALUE *args, int kw_splat) {
@@ -33,7 +36,10 @@ VALUE rb_funcallv_kw(VALUE object, ID name, int args_count, const VALUE *args, i
3336
}
3437

3538
VALUE rb_funcallv_public(VALUE object, ID name, int args_count, const VALUE *args) {
36-
return RUBY_CEXT_INVOKE("rb_funcallv_public", object, ID2SYM(name), rb_ary_new4(args_count, args));
39+
return rb_tr_wrap(polyglot_invoke(RUBY_CEXT, "rb_funcallv_public",
40+
rb_tr_unwrap(object),
41+
rb_tr_unwrap(ID2SYM(name)),
42+
polyglot_from_VALUE_array(args, args_count)));
3743
}
3844

3945
VALUE rb_apply(VALUE object, ID name, VALUE args) {
@@ -83,7 +89,11 @@ VALUE rb_yield(VALUE value) {
8389
}
8490

8591
VALUE rb_funcall_with_block(VALUE recv, ID mid, int argc, const VALUE *argv, VALUE pass_procval) {
86-
return RUBY_CEXT_INVOKE("rb_funcall_with_block", recv, ID2SYM(mid), rb_ary_new4(argc, argv), pass_procval);
92+
return rb_tr_wrap(polyglot_invoke(RUBY_CEXT, "rb_funcall_with_block",
93+
rb_tr_unwrap(recv),
94+
rb_tr_unwrap(ID2SYM(mid)),
95+
polyglot_from_VALUE_array(argv, argc),
96+
rb_tr_unwrap(pass_procval)));
8797
}
8898

8999
VALUE rb_yield_splat(VALUE values) {

‎src/main/java/org/truffleruby/cext/CExtNodes.java

Lines changed: 54 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.jcodings.specific.USASCIIEncoding;
2020
import org.jcodings.specific.UTF8Encoding;
2121
import org.truffleruby.Layouts;
22+
import org.truffleruby.RubyContext;
2223
import org.truffleruby.builtins.CoreMethod;
2324
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
2425
import org.truffleruby.builtins.CoreMethodNode;
@@ -28,6 +29,7 @@
2829
import org.truffleruby.builtins.YieldingCoreMethodNode;
2930
import org.truffleruby.cext.CExtNodesFactory.CallWithCExtLockNodeFactory;
3031
import org.truffleruby.cext.CExtNodesFactory.StringToNativeNodeGen;
32+
import org.truffleruby.cext.UnwrapNode.UnwrapCArrayNode;
3133
import org.truffleruby.collections.ConcurrentOperations;
3234
import org.truffleruby.core.CoreLibrary;
3335
import org.truffleruby.core.MarkingService.ExtensionCallStack;
@@ -173,6 +175,27 @@ protected int getCacheLimit() {
173175
}
174176
}
175177

178+
public static Object sendWithoutCExtLock(RubyContext context, Object receiver, RubySymbol method, Object block,
179+
DispatchNode dispatchNode, ConditionProfile ownedProfile, Object[] args, Node currentNode) {
180+
if (context.getOptions().CEXT_LOCK) {
181+
final ReentrantLock lock = context.getCExtensionsLock();
182+
boolean owned = ownedProfile.profile(lock.isHeldByCurrentThread());
183+
184+
if (owned) {
185+
MutexOperations.unlockInternal(lock);
186+
}
187+
try {
188+
return dispatchNode.callWithBlock(receiver, method.getString(), block, args);
189+
} finally {
190+
if (owned) {
191+
MutexOperations.internalLockEvenWithException(context, lock, currentNode);
192+
}
193+
}
194+
} else {
195+
return dispatchNode.callWithBlock(receiver, method.getString(), block, args);
196+
}
197+
}
198+
176199
@Primitive(name = "send_without_cext_lock")
177200
public abstract static class SendWithoutCExtLockNode extends PrimitiveArrayArgumentsNode {
178201
@Specialization
@@ -181,53 +204,35 @@ protected Object sendWithoutCExtLock(Object receiver, RubySymbol method, RubyArr
181204
@Cached DispatchNode dispatchNode,
182205
@Cached ConditionProfile ownedProfile) {
183206
final Object[] args = arrayToObjectArrayNode.executeToObjectArray(argsArray);
207+
return CExtNodes
208+
.sendWithoutCExtLock(getContext(), receiver, method, block, dispatchNode, ownedProfile, args, this);
209+
}
184210

185-
if (getContext().getOptions().CEXT_LOCK) {
186-
final ReentrantLock lock = getContext().getCExtensionsLock();
187-
boolean owned = ownedProfile.profile(lock.isHeldByCurrentThread());
211+
}
188212

189-
if (owned) {
190-
MutexOperations.unlockInternal(lock);
191-
}
192-
try {
193-
return dispatchNode.callWithBlock(receiver, method.getString(), block, args);
194-
} finally {
195-
if (owned) {
196-
MutexOperations.internalLockEvenWithException(getContext(), lock, this);
197-
}
198-
}
199-
} else {
200-
return dispatchNode.callWithBlock(receiver, method.getString(), block, args);
201-
}
213+
@Primitive(name = "send_argv_without_cext_lock")
214+
public abstract static class SendARGVWithoutCExtLockNode extends PrimitiveArrayArgumentsNode {
215+
@Specialization
216+
protected Object sendWithoutCExtLock(Object receiver, RubySymbol method, Object argv, Object block,
217+
@Cached UnwrapCArrayNode unwrapCArrayNode,
218+
@Cached DispatchNode dispatchNode,
219+
@Cached ConditionProfile ownedProfile) {
220+
final Object[] args = unwrapCArrayNode.execute(argv);
221+
return CExtNodes
222+
.sendWithoutCExtLock(getContext(), receiver, method, block, dispatchNode, ownedProfile, args, this);
202223
}
203224
}
204225

205-
@Primitive(name = "public_send_without_cext_lock")
206-
public abstract static class PublicSendWithoutCExtLockNode extends PrimitiveArrayArgumentsNode {
226+
@Primitive(name = "public_send_argv_without_cext_lock", lowerFixnum = 2)
227+
public abstract static class PublicSendARGVWithoutCExtLockNode extends PrimitiveArrayArgumentsNode {
207228
@Specialization
208-
protected Object publicSendWithoutLock(Object receiver, RubySymbol method, RubyArray argsArray, Object block,
209-
@Cached ArrayToObjectArrayNode arrayToObjectArrayNode,
229+
protected Object publicSendWithoutLock(Object receiver, RubySymbol method, Object argv, Object block,
230+
@Cached UnwrapCArrayNode unwrapCArrayNode,
210231
@Cached(parameters = "PUBLIC") DispatchNode dispatchNode,
211232
@Cached ConditionProfile ownedProfile) {
212-
final Object[] args = arrayToObjectArrayNode.executeToObjectArray(argsArray);
213-
214-
if (getContext().getOptions().CEXT_LOCK) {
215-
final ReentrantLock lock = getContext().getCExtensionsLock();
216-
boolean owned = ownedProfile.profile(lock.isHeldByCurrentThread());
217-
218-
if (owned) {
219-
MutexOperations.unlockInternal(lock);
220-
}
221-
try {
222-
return dispatchNode.callWithBlock(receiver, method.getString(), block, args);
223-
} finally {
224-
if (owned) {
225-
MutexOperations.internalLockEvenWithException(getContext(), lock, this);
226-
}
227-
}
228-
} else {
229-
return dispatchNode.callWithBlock(receiver, method.getString(), block, args);
230-
}
233+
final Object[] args = unwrapCArrayNode.execute(argv);
234+
return CExtNodes
235+
.sendWithoutCExtLock(getContext(), receiver, method, block, dispatchNode, ownedProfile, args, this);
231236
}
232237
}
233238

@@ -1609,4 +1614,14 @@ protected Object checkSymbolCStr(Object string,
16091614
return sym == null ? nil : sym;
16101615
}
16111616
}
1617+
1618+
@CoreMethod(names = "rb_ary_new_from_values", onSingleton = true, required = 1)
1619+
public abstract static class RbAryNewFromValues extends CoreMethodArrayArgumentsNode {
1620+
@Specialization
1621+
protected RubyArray rbAryNewFromValues(Object cArray,
1622+
@Cached UnwrapCArrayNode unwrapCArrayNode) {
1623+
final Object[] values = unwrapCArrayNode.execute(cArray);
1624+
return createArray(values);
1625+
}
1626+
}
16121627
}

‎src/main/java/org/truffleruby/cext/UnwrapNode.java

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@
1414
import static org.truffleruby.cext.ValueWrapperManager.UNDEF_HANDLE;
1515

1616
import com.oracle.truffle.api.CompilerDirectives;
17+
import com.oracle.truffle.api.dsl.Bind;
18+
import com.oracle.truffle.api.interop.InvalidArrayIndexException;
19+
import com.oracle.truffle.api.nodes.ExplodeLoop;
20+
import com.oracle.truffle.api.nodes.LoopNode;
21+
import com.oracle.truffle.api.profiles.LoopConditionProfile;
1722
import org.truffleruby.RubyContext;
1823
import org.truffleruby.RubyLanguage;
1924
import org.truffleruby.cext.UnwrapNodeGen.NativeToWrapperNodeGen;
@@ -37,13 +42,9 @@
3742
import com.oracle.truffle.api.profiles.BranchProfile;
3843

3944
@GenerateUncached
40-
@ImportStatic({ ValueWrapperManager.class })
45+
@ImportStatic(ValueWrapperManager.class)
4146
public abstract class UnwrapNode extends RubyBaseNode {
4247

43-
public static UnwrapNode create() {
44-
return UnwrapNodeGen.create();
45-
}
46-
4748
@GenerateUncached
4849
@ImportStatic(ValueWrapperManager.class)
4950
public abstract static class UnwrapNativeNode extends RubyBaseNode {
@@ -154,7 +155,7 @@ public static NativeToWrapperNode create() {
154155
}
155156
}
156157

157-
@ImportStatic({ ValueWrapperManager.class })
158+
@ImportStatic(ValueWrapperManager.class)
158159
public abstract static class ToWrapperNode extends RubyContextNode {
159160

160161
public abstract ValueWrapper execute(Object value);
@@ -194,6 +195,65 @@ protected int getCacheLimit() {
194195
}
195196
}
196197

198+
@ImportStatic(ValueWrapperManager.class)
199+
public abstract static class UnwrapCArrayNode extends RubyContextNode {
200+
201+
public abstract Object[] execute(Object cArray);
202+
203+
@ExplodeLoop
204+
@Specialization(
205+
guards = { "size == cachedSize", "cachedSize <= MAX_EXPLODE_SIZE" },
206+
limit = "getIdentityCacheLimit()")
207+
protected Object[] unwrapCArrayExplode(Object cArray,
208+
@CachedLibrary("cArray") InteropLibrary interop,
209+
@Bind("getArraySize(cArray, interop)") int size,
210+
@Cached("size") int cachedSize,
211+
@Cached UnwrapNode unwrapNode) {
212+
final Object[] store = new Object[cachedSize];
213+
for (int i = 0; i < cachedSize; i++) {
214+
final Object cValue = readArrayElement(cArray, interop, i);
215+
store[i] = unwrapNode.execute(cValue);
216+
}
217+
return store;
218+
}
219+
220+
@Specialization(replaces = "unwrapCArrayExplode", limit = "getDefaultCacheLimit()")
221+
protected Object[] unwrapCArray(Object cArray,
222+
@CachedLibrary("cArray") InteropLibrary interop,
223+
@Bind("getArraySize(cArray, interop)") int size,
224+
@Cached UnwrapNode unwrapNode,
225+
@Cached LoopConditionProfile loopProfile) {
226+
final Object[] store = new Object[size];
227+
loopProfile.profileCounted(size);
228+
for (int i = 0; loopProfile.inject(i < size); i++) {
229+
final Object cValue = readArrayElement(cArray, interop, i);
230+
store[i] = unwrapNode.execute(cValue);
231+
}
232+
LoopNode.reportLoopCount(this, size);
233+
return store;
234+
}
235+
236+
protected static int getArraySize(Object cArray, InteropLibrary interop) {
237+
try {
238+
return Math.toIntExact(interop.getArraySize(cArray));
239+
} catch (UnsupportedMessageException | ArithmeticException e) {
240+
throw CompilerDirectives.shouldNotReachHere(e);
241+
}
242+
}
243+
244+
private static Object readArrayElement(Object cArray, InteropLibrary interop, int i) {
245+
try {
246+
return interop.readArrayElement(cArray, i);
247+
} catch (UnsupportedMessageException | InvalidArrayIndexException e) {
248+
throw CompilerDirectives.shouldNotReachHere(e);
249+
}
250+
}
251+
}
252+
253+
public static UnwrapNode create() {
254+
return UnwrapNodeGen.create();
255+
}
256+
197257
public abstract Object execute(Object value);
198258

199259
@Specialization(guards = "!isTaggedLong(value.getHandle())")

‎test/truffle/cexts/backtraces/bin/backtraces

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def callback
2020
end
2121

2222
def ruby_call
23-
RB_FUNCALLV.call(Truffle::CExt.test_cext_wrap(self), Truffle::CExt.test_cext_wrap(:callback), 0, nil)
23+
RB_FUNCALLV.call(Truffle::CExt.test_cext_wrap(self), Truffle::CExt.test_cext_wrap(:callback), 0, [])
2424
end
2525

2626
def top

‎test/truffle/cexts/backtraces/expected.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
Test error in Ruby => C ext support => Ruby
22
/bin/backtraces:19:in `callback': Ruby callback error (RuntimeError)
3-
from /lib/truffle/truffle/cext.rb:n:in `rb_funcall'
43
from /lib/truffle/truffle/cext.rb:n:in `rb_funcallv'
54
from /src/main/c/cext/call.c:n:in `rb_funcallv'
65
from /bin/backtraces:23:in `ruby_call'

0 commit comments

Comments
 (0)