-
Notifications
You must be signed in to change notification settings - Fork 70
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
FastProtocolBridge #2106
FastProtocolBridge #2106
Conversation
private lateinit var changesSinkService: ChangesSinkService | ||
private lateinit var sendChanges: (service: ChangesSinkService, args: List<*>) -> Any? | ||
|
||
public override val widgetSystem: WidgetSystem<Unit> = |
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.
Too much duplication here!
): ProtocolBridge = FastProtocolBridge(json, hostVersion, widgetSystemFactory, mismatchHandler) | ||
|
||
@OptIn(RedwoodCodegenApi::class) | ||
private class FastProtocolBridge( |
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 super dislike this name, but I super don't care when private
!
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.
Can fix!
61deed6
to
477ba9f
Compare
* Confirm that [FastProtocolBridge] behaves the same as [DefaultProtocolBridge]. | ||
*/ | ||
@OptIn(ExperimentalSerializationApi::class, RedwoodCodegenApi::class) | ||
class FastProtocolBridgeTest { |
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.
FYI @JakeWharton this is new!
id: Id, | ||
tag: PropertyTag, | ||
value: UInt, | ||
) |
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.
This is new! And it’s quite unfortunate!
I don’t think we need to implement something general purpose for other kotlinx.serialization types. UInt is something we’re already using in Cash App, so we’re pinned here.
This is a new implementation of ProtocolBridge that uses Zipline's new asDynamicFunction() API. It's dramatically faster - in one measurement we went from spending ~18% of our samples on serialization to ~3% of our samples.
81c5a56
to
7587680
Compare
This is a new implementation of ProtocolBridge that uses Zipline's new asDynamicFunction() API. It's dramatically faster - in one measurement we went from spending ~18% of our samples on serialization to ~3% of our samples.