-
Notifications
You must be signed in to change notification settings - Fork 93
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
Renewed function invocation workflow to avoid metadata downloads #660
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Finally, an important test is succeeding
|
justcoon
approved these changes
Jul 31, 2024
I will address @vigoo 's comments in a separate PR. I think we have come a long way by this time, and would like to merge a working version. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Optimizations around Component Metadata Downloads
worker-service
mainly consist of two components. Aservice
which acts as the entry point to invoke worker functions andworker-gateway
. Theservice
is mainly used bygolem-cli
as well asworker-gateway
.worker-service
also consist ofgrpc
service which is used byworker-executor
mainly.Any worker invocation (be it gateway call, or direct invocation through CLI, or the call back from worker-proxy in worker-executor), all of them led to a metadata download which in turn calls component-service. This document and PR describes how we avoided this, without compromising some of the type-safety that we have in golem-rib evaluation, and hopefully increase performance (as we reduce the number of RPC calls to component-service)
Note that, this optimization is not just done for worker-gateway (or worker-bridge).
New invoke functions and Optimisations
wasm_rpc::Val
internally before calling worker-executorTypeAnnotatedValue
so that there isn't a need to download the metadata at client site to interpretVec<wasm_rpc::Val>
Vec<Val>
as input as well asVec<PreciseJson>
, and one returningVec<Val>
as output and the other aTypeAnnotatedValue
which can be converted to JSONempty
array was failing when converted to TypeAnnotatedValue. This is fixed in a separate PR. Take a look at this one: Fix resource drop functionality in worker-executor - #717 #718When evaluating Rib, we used to download the metdata for mainly two purposes.
{}
could be an empty flag or an empty record etc.Function Name Parsing -
ParsedFunctionName
andDisplay
instanceParsedFunctionName
. However the functions exposed by worker-service as well as GRPC interface of worker-executor, all work with function name asString
. To make lesser changes in this PR, I have introduced a display instane for ParsedFunctionName, and worker-bridge simply reuses the existing functions without introducing a new function that acceptsParsedFunctionName
.ParsedFunctionName
, one during Rib evaluation, and one during execution of the worker function by worker-executor.Please find the follow up ticket raised based on a review comment from @vigoo :
#724