Skip to content

Commit 1e3e006

Browse files
eregoneregon
committed
Unwrap VALUE* args for rb_funcallv* and rb_funcall_with_block directly
* Without going through rb_ary_new4() which is quite some overhead for this.
1 parent 238d596 commit 1e3e006

File tree

8 files changed

+135
-56
lines changed

8 files changed

+135
-56
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Performance:
4646
* `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)
4747
* Significantly improved performance of `Time#strftime` for common formats (#2361, @wildmaples, @chrisseaton).
4848
* Faster solution for lazy integer length (#2365, @lemire, @chrisseaton).
49+
* Speedup `rb_funcallv*()` by directly unwrapping the C arguments array instead of going through a Ruby `Array` (#2089).
4950

5051
Changes:
5152

‎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: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -865,16 +865,23 @@ 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+
# see #call_with_thread_locally_stored_block
878+
thread_local_block = Thread.current[:__C_BLOCK__]
879+
Thread.current[:__C_BLOCK__] = nil
880+
begin
881+
Primitive.send_argv_without_cext_lock(recv, meth, argv, thread_local_block)
882+
ensure
883+
Thread.current[:__C_BLOCK__] = thread_local_block
884+
end
878885
end
879886

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

‎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: 44 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

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

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
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;
1720
import org.truffleruby.RubyContext;
1821
import org.truffleruby.RubyLanguage;
1922
import org.truffleruby.cext.UnwrapNodeGen.NativeToWrapperNodeGen;
@@ -37,13 +40,9 @@
3740
import com.oracle.truffle.api.profiles.BranchProfile;
3841

3942
@GenerateUncached
40-
@ImportStatic({ ValueWrapperManager.class })
43+
@ImportStatic(ValueWrapperManager.class)
4144
public abstract class UnwrapNode extends RubyBaseNode {
4245

43-
public static UnwrapNode create() {
44-
return UnwrapNodeGen.create();
45-
}
46-
4746
@GenerateUncached
4847
@ImportStatic(ValueWrapperManager.class)
4948
public abstract static class UnwrapNativeNode extends RubyBaseNode {
@@ -154,7 +153,7 @@ public static NativeToWrapperNode create() {
154153
}
155154
}
156155

157-
@ImportStatic({ ValueWrapperManager.class })
156+
@ImportStatic(ValueWrapperManager.class)
158157
public abstract static class ToWrapperNode extends RubyContextNode {
159158

160159
public abstract ValueWrapper execute(Object value);
@@ -194,6 +193,62 @@ protected int getCacheLimit() {
194193
}
195194
}
196195

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

199254
@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)