-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf(ext/ffi): leverage V8 Fast Calls #15125
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} | ||
|
||
let sym = Box::leak(sym); | ||
let builder = v8::FunctionTemplate::builder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing from v8::Function
to v8::FunctionTemplate
means that the functions become un-collectable by the GC. Should there be a possible option to opt out from the optimization?
NativeType::F32 => fast_api::Type::Float32, | ||
NativeType::F64 => fast_api::Type::Float64, | ||
NativeType::Void => fast_api::Type::Void, | ||
NativeType::Function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess theoretically we could support int64_t
and uint64_t
argument types, meaning also pointers. The slow functions already have (or at least had) support for reading a number as a pointer value. Allowing uint64_t
to stand in here for pointers et al. would mean that users could opt in to the Fast Call API by explicitly using Number(UnsafePointer.of(typedArray))
for pointer values. Of course, users would also need to take care to not lose any data through this.
.map(|t| t.into()) | ||
.collect::<Vec<_>>(); | ||
if args.is_empty() { | ||
args.push(fast_api::Type::V8Value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Fast API not support empty arguments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently needs the receiver V8Value for empty calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. That's odd and dumb. Did you happen to try with a single void argument? That would at least make a little sense with how C functions that absolutely do not accept parameters are declared with a single void argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Void arguments trigger unreachable paths in V8
100x faster calls into FFI