Skip to content
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

Callback interface speedup #1497

Merged
merged 10 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Optimize callback interface methods that return void
If a callback interface method has a void return, there's no need to
overwrite the value in the output buffer.  Rust has already initialized
an empty buffer.  Also, there's no need to construct a RustBuffer in the
first place.

Refactored the callback interface code to make this possible without too
much spaghettification.

This gives us a decent speedup on Python/Kotlin:

Benchmarking callbacks/python-callbacks-void-return
Benchmarking callbacks/python-callbacks-void-return: Warming up for 3.0000 s
Benchmarking callbacks/python-callbacks-void-return: Collecting 100 samples in estimated 10.050 s (813k iterations)
Benchmarking callbacks/python-callbacks-void-return: Analyzing
callbacks/python-callbacks-void-return
                        time:   [12.184 µs 12.219 µs 12.266 µs]
                        change: [-26.774% -26.526% -26.286%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  2 (2.00%) high mild
  7 (7.00%) high severe

Benchmarking callbacks/kotlin-callbacks-void-return
Benchmarking callbacks/kotlin-callbacks-void-return: Warming up for 3.0000 s
Benchmarking callbacks/kotlin-callbacks-void-return: Collecting 100 samples in estimated 10.007 s (1.9M iterations)
Benchmarking callbacks/kotlin-callbacks-void-return: Analyzing
callbacks/kotlin-callbacks-void-return
                        time:   [5.1736 µs 5.2086 µs 5.2430 µs]
                        change: [-32.627% -26.167% -20.263%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  3 (3.00%) low severe
  6 (6.00%) low mild
  1 (1.00%) high mild
  3 (3.00%) high severe
  • Loading branch information
bendk committed Mar 23, 2023
commit 14e0bddcca89306b895f9d4294f449075335c40d
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,7 @@ internal class {{ foreign_callback }} : ForeignCallback {
// Call the method, write to outBuf and return a status code
// See docs of ForeignCallback in `uniffi/src/ffi/foreigncallbacks.rs` for info
try {
{%- match meth.throws_type() %}
{%- when Some(error_type) %}
try {
val buffer = this.{{ method_name }}(cb, args)
// Success
outBuf.setValue(buffer)
1
} catch (e: {{ error_type|type_name }}) {
// Expected error
val buffer = {{ error_type|ffi_converter_name }}.lowerIntoRustBuffer(e)
outBuf.setValue(buffer)
-2
}
{%- else %}
val buffer = this.{{ method_name }}(cb, args)
// Success
outBuf.setValue(buffer)
1
{%- endmatch %}
this.{{ method_name }}(cb, args, outBuf)
} catch (e: Throwable) {
// Unexpected error
try {
Expand Down Expand Up @@ -84,32 +66,50 @@ internal class {{ foreign_callback }} : ForeignCallback {

{% for meth in cbi.methods() -%}
{% let method_name = format!("invoke_{}", meth.name())|fn_name %}
private fun {{ method_name }}(kotlinCallbackInterface: {{ type_name }}, @Suppress("UNUSED_PARAMETER")args: RustBuffer.ByReference): RustBuffer.ByValue {
{#- Unpacking args from the RustBuffer #}
{%- if meth.arguments().len() != 0 -%}
{#- Calling the concrete callback object #}
val buf = args.asByteBuffer() ?: throw InternalException("No ByteBuffer in RustBuffer; this is a Uniffi bug")
return kotlinCallbackInterface.{{ meth.name()|fn_name }}(
{% for arg in meth.arguments() -%}
{{ arg|read_fn }}(buf)
@Suppress("UNUSED_PARAMETER")
private fun {{ method_name }}(kotlinCallbackInterface: {{ type_name }}, args: RustBuffer.ByReference, outBuf: RustBufferByReference): Int {
{%- if meth.arguments().len() > 0 %}
val argsBuf = args.asByteBuffer() ?: throw InternalException("No ByteBuffer in RustBuffer; this is a Uniffi bug")
{%- endif %}

{%- match meth.return_type() %}
{%- when Some with (return_type) %}
fun makeCall() : Int {
val returnValue = kotlinCallbackInterface.{{ meth.name()|fn_name }}(
{%- for arg in meth.arguments() %}
{{ arg|read_fn }}(argsBuf)
{% if !loop.last %}, {% endif %}
{%- endfor %}
)
outBuf.setValue({{ return_type|ffi_converter_name }}.lowerIntoRustBuffer(returnValue))
return 1
}
{%- when None %}
fun makeCall() : Int {
kotlinCallbackInterface.{{ meth.name()|fn_name }}(
{%- for arg in meth.arguments() %}
{{ arg|read_fn }}(argsBuf)
{%- if !loop.last %}, {% endif %}
{% endfor -%}
)
{% else %}
return kotlinCallbackInterface.{{ meth.name()|fn_name }}()
{% endif -%}
{%- endfor %}
)
return 1
}
{%- endmatch %}

{#- Packing up the return value into a RustBuffer #}
{%- match meth.return_type() -%}
{%- when Some with (return_type) -%}
.let {
{{ return_type|ffi_converter_name }}.lowerIntoRustBuffer(it)
}
{%- else -%}
.let { RustBuffer.ByValue() }
{%- match meth.throws_type() %}
{%- when None %}
fun makeCallAndHandleError() : Int = makeCall()
{%- when Some(error_type) %}
fun makeCallAndHandleError() : Int = try {
makeCall()
} catch (e: {{ error_type|type_name }}) {
// Expected error, serialize it into outBuf
outBuf.setValue({{ error_type|ffi_converter_name }}.lowerIntoRustBuffer(e))
-2
}
{%- endmatch %}
// TODO catch errors and report them back to Rust.
// https://github.com/mozilla/uniffi-rs/issues/351

return makeCallAndHandleError()
}
{% endfor %}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,48 @@ def {{ meth.name()|fn_name }}({% call py::arg_list_decl(meth) %}):
def py_{{ foreign_callback }}(handle, method, args, buf_ptr):
{% for meth in cbi.methods() -%}
{% let method_name = format!("invoke_{}", meth.name())|fn_name %}
def {{ method_name }}(python_callback, args):
def {{ method_name }}(python_callback, args, buf_ptr):
{#- Unpacking args from the RustBuffer #}
{%- if meth.arguments().len() != 0 -%}
{#- Calling the concrete callback object #}
with args.contents.readWithStream() as buf:
rval = python_callback.{{ meth.name()|fn_name }}(
{% for arg in meth.arguments() -%}
{{ arg|read_fn }}(buf)
{%- if !loop.last %}, {% endif %}
{% endfor -%}
)
{% else %}
rval = python_callback.{{ meth.name()|fn_name }}()
{% endif -%}
def makeCall():
{%- if meth.arguments().len() != 0 -%}
{#- Calling the concrete callback object #}
with args.contents.readWithStream() as buf:
return python_callback.{{ meth.name()|fn_name }}(
{% for arg in meth.arguments() -%}
{{ arg|read_fn }}(buf)
{%- if !loop.last %}, {% endif %}
{% endfor -%}
)
{%- else %}
return python_callback.{{ meth.name()|fn_name }}()
{%- endif %}

def makeCallAndHandleReturn():
{%- match meth.return_type() %}
{%- when Some(return_type) %}
rval = makeCall()
with RustBuffer.allocWithBuilder() as builder:
{{ return_type|write_fn }}(rval, builder)
buf_ptr[0] = builder.finalize()
{%- when None %}
makeCall()
{%- endmatch %}
return 1

{%- match meth.throws_type() %}
{%- when None %}
return makeCallAndHandleReturn()
{%- when Some(err) %}
try:
return makeCallAndHandleReturn()
except {{ err|type_name }} as e:
# Catch errors declared in the UDL file
with RustBuffer.allocWithBuilder() as builder:
{{ err|write_fn }}(e, builder)
buf_ptr[0] = builder.finalize()
return -2
{%- endmatch %}

{#- Packing up the return value into a RustBuffer #}
{%- match meth.return_type() -%}
{%- when Some with (return_type) -%}
with RustBuffer.allocWithBuilder() as builder:
{{ return_type|write_fn }}(rval, builder)
return builder.finalize()
{%- else -%}
return RustBuffer.alloc(0)
{% endmatch -%}
{% endfor %}

cb = {{ ffi_converter_name }}.lift(handle)
Expand All @@ -57,23 +75,7 @@ def {{ method_name }}(python_callback, args):
# Call the method and handle any errors
# See docs of ForeignCallback in `uniffi/src/ffi/foreigncallbacks.rs` for details
try:
{%- match meth.throws_type() %}
{%- when Some(err) %}
try:
# Successful return
buf_ptr[0] = {{ method_name }}(cb, args)
return 1
except {{ err|type_name }} as e:
# Catch errors declared in the UDL file
with RustBuffer.allocWithBuilder() as builder:
{{ err|write_fn }}(e, builder)
buf_ptr[0] = builder.finalize()
return -2
{%- else %}
# Successful return
buf_ptr[0] = {{ method_name }}(cb, args)
return 1
{%- endmatch %}
return {{ method_name }}(cb, args, buf_ptr)
except BaseException as e:
# Catch unexpected errors
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,87 +17,87 @@ public protocol {{ type_name }} : AnyObject {
// The ForeignCallback that is passed to Rust.
fileprivate let {{ foreign_callback }} : ForeignCallback =
{ (handle: UniFFICallbackHandle, method: Int32, args: UnsafePointer<RustBuffer>, out_buf: UnsafeMutablePointer<RustBuffer>) -> Int32 in
{% for meth in cbi.methods() -%}
{%- let method_name = format!("invoke_{}", meth.name())|fn_name -%}
{% for meth in cbi.methods() -%}
{%- let method_name = format!("invoke_{}", meth.name())|fn_name %}

func {{ method_name }}(_ swiftCallbackInterface: {{ type_name }}, _ args: UnsafePointer<RustBuffer>) throws -> RustBuffer {
{#- Unpacking args from the RustBuffer #}
{%- if meth.arguments().len() != 0 -%}
{#- Calling the concrete callback object #}
func {{ method_name }}(_ swiftCallbackInterface: {{ type_name }}, _ args: UnsafePointer<RustBuffer>, _ out_buf: UnsafeMutablePointer<RustBuffer>) throws -> Int32 {
{%- if meth.arguments().len() > 0 %}
var reader = createReader(data: Data(rustBuffer: args.pointee))
{%- endif %}

var reader = createReader(data: Data(rustBuffer: args.pointee))
{% if meth.return_type().is_some() %}let result = {% endif -%}
{% if meth.throws() %}try {% endif -%}
swiftCallbackInterface.{{ meth.name()|fn_name }}(
{%- match meth.return_type() %}
{%- when Some(return_type) %}
func makeCall() throws -> Int32 {
let result = try swiftCallbackInterface.{{ meth.name()|fn_name }}(
{% for arg in meth.arguments() -%}
{% if !config.omit_argument_labels() %}{{ arg.name()|var_name }}: {% endif %} try {{ arg|read_fn }}(from: &reader)
{%- if !loop.last %}, {% endif %}
{% endfor -%}
)
{% else %}
{% if meth.return_type().is_some() %}let result = {% endif -%}
{% if meth.throws() %}try {% endif -%}
swiftCallbackInterface.{{ meth.name()|fn_name }}()
{% endif -%}

{#- Packing up the return value into a RustBuffer #}
{%- match meth.return_type() -%}
{%- when Some with (return_type) -%}
var writer = [UInt8]()
{{ return_type|write_fn }}(result, into: &writer)
return RustBuffer(bytes: writer)
{%- else -%}
return RustBuffer()
{% endmatch -%}
// TODO catch errors and report them back to Rust.
// https://github.com/mozilla/uniffi-rs/issues/351

var writer = [UInt8]()
{{ return_type|write_fn }}(result, into: &writer)
out_buf.pointee = RustBuffer(bytes: writer)
return 1
}
{% endfor %}
{%- when None %}
func makeCall() throws -> Int32 {
try swiftCallbackInterface.{{ meth.name()|fn_name }}(
{% for arg in meth.arguments() -%}
{% if !config.omit_argument_labels() %}{{ arg.name()|var_name }}: {% endif %} try {{ arg|read_fn }}(from: &reader)
{%- if !loop.last %}, {% endif %}
{% endfor -%}
)
return 1
}
{%- endmatch %}

let cb: {{ cbi|type_name }}
{%- match meth.throws_type() %}
{%- when None %}
return try makeCall()
{%- when Some(error_type) %}
do {
cb = try {{ ffi_converter_name }}.lift(handle)
} catch {
out_buf.pointee = {{ Type::String.borrow()|lower_fn }}("{{ cbi.name() }}: Invalid handle")
return -1
return try makeCall()
} catch let error as {{ error_type|type_name }} {
out_buf.pointee = {{ error_type|lower_fn }}(error)
return -2
}
{%- endmatch %}
}
{%- endfor %}


switch method {
case IDX_CALLBACK_FREE:
{{ ffi_converter_name }}.drop(handle: handle)
// No return value.
// See docs of ForeignCallback in `uniffi/src/ffi/foreigncallbacks.rs`
return 0
{% for meth in cbi.methods() -%}
{% let method_name = format!("invoke_{}", meth.name())|fn_name -%}
case {{ loop.index }}:
do {
out_buf.pointee = try {{ method_name }}(cb, args)
// Value written to out buffer.
// See docs of ForeignCallback in `uniffi/src/ffi/foreigncallbacks.rs`
return 1
{%- match meth.throws_type() %}
{%- when Some(error_type) %}
} catch let error as {{ error_type|type_name }} {
out_buf.pointee = {{ error_type|lower_fn }}(error)
return -2
{%- else %}
{%- endmatch %}
} catch let error {
out_buf.pointee = {{ Type::String.borrow()|lower_fn }}(String(describing: error))
return -1
}
{% endfor %}
// This should never happen, because an out of bounds method index won't
// ever be used. Once we can catch errors, we should return an InternalError.
// https://github.com/mozilla/uniffi-rs/issues/351
default:
// An unexpected error happened.
// See docs of ForeignCallback in `uniffi/src/ffi/foreigncallbacks.rs`
switch method {
case IDX_CALLBACK_FREE:
{{ ffi_converter_name }}.drop(handle: handle)
// No return value.
// See docs of ForeignCallback in `uniffi/src/ffi/foreigncallbacks.rs`
return 0
{% for meth in cbi.methods() -%}
{% let method_name = format!("invoke_{}", meth.name())|fn_name -%}
case {{ loop.index }}:
let cb: {{ cbi|type_name }}
do {
cb = try {{ ffi_converter_name }}.lift(handle)
} catch {
out_buf.pointee = {{ Type::String.borrow()|lower_fn }}("{{ cbi.name() }}: Invalid handle")
return -1
}
}
do {
return try {{ method_name }}(cb, args, out_buf)
} catch let error {
out_buf.pointee = {{ Type::String.borrow()|lower_fn }}(String(describing: error))
return -1
}
{% endfor %}
// This should never happen, because an out of bounds method index won't
// ever be used. Once we can catch errors, we should return an InternalError.
// https://github.com/mozilla/uniffi-rs/issues/351
default:
// An unexpected error happened.
// See docs of ForeignCallback in `uniffi/src/ffi/foreigncallbacks.rs`
return -1
}
}

// FfiConverter protocol for callback interfaces
fileprivate struct {{ ffi_converter_name }} {
Expand Down