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

Support dynamic resource parameters in Rib #960

Merged
merged 33 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
68e651d
Support dynamic resource parameters in Rib
afsalthaj Sep 20, 2024
ed7ebe7
Bring grpc types
afsalthaj Sep 23, 2024
a7e907c
Add IR proto
afsalthaj Sep 23, 2024
385bec7
Add more rib
afsalthaj Sep 23, 2024
caadf4c
Add type
afsalthaj Sep 23, 2024
2ff4124
Add Expr
afsalthaj Sep 23, 2024
4bf530a
Add Expr
afsalthaj Sep 23, 2024
486d07e
Fix test cases
afsalthaj Sep 23, 2024
318b48a
Try to fix function name
afsalthaj Sep 23, 2024
632e910
Fix tests
afsalthaj Sep 23, 2024
a538b65
Reformat
afsalthaj Sep 23, 2024
1fc4453
Fix clippy
afsalthaj Sep 23, 2024
9d14333
Refactor
afsalthaj Sep 23, 2024
e833175
Merge branch 'main' into dynamic_resource_parameters
afsalthaj Sep 23, 2024
ae3058b
Use function name
afsalthaj Sep 24, 2024
e4d8f15
Merge remote-tracking branch 'origin/dynamic_resource_parameters' int…
afsalthaj Sep 24, 2024
74d8945
Write test for indexed function
afsalthaj Sep 24, 2024
5f69c0c
Make sure resource params are created
afsalthaj Sep 24, 2024
39ff6ed
Merge branch 'main' into dynamic_resource_parameters
afsalthaj Sep 24, 2024
d91f000
Delete comments
afsalthaj Sep 24, 2024
287cc72
Merge remote-tracking branch 'origin/dynamic_resource_parameters' int…
afsalthaj Sep 24, 2024
44d13a4
Remove clone
afsalthaj Sep 24, 2024
f10154f
Reformat code
afsalthaj Sep 24, 2024
07bbf7e
Add more tests
afsalthaj Sep 24, 2024
2ffcfff
Add more tests
afsalthaj Sep 24, 2024
ab98630
Remove the need of serde for RibByteCode
afsalthaj Sep 24, 2024
6149dd4
Use field names in instruction set instead of anonymouse types
afsalthaj Sep 24, 2024
170dbcf
Check if its part of resource than indexed_resource
afsalthaj Sep 24, 2024
3eeeb3a
Merge branch 'main' into dynamic_resource_parameters
afsalthaj Sep 24, 2024
528bb79
Make grpc types backward compatible
afsalthaj Sep 24, 2024
22e2702
Merge remote-tracking branch 'origin/dynamic_resource_parameters' int…
afsalthaj Sep 24, 2024
61d3b82
Reformat code
afsalthaj Sep 24, 2024
eb2f113
Add tests
afsalthaj Sep 24, 2024
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
43 changes: 42 additions & 1 deletion golem-api-grpc/proto/golem/rib/expr.proto
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ message CallExpr {

message InvocationName {
oneof name {
golem.rib.ParsedFunctionName parsed = 1;
golem.rib.DynamicParsedFunctionName parsed = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how? These are grpc types only for internal persistence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, discard my question. In terms of backward compatibility, well the function name is different now because has Exprs in it, and it's a different structure. I need to test backward compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is internal persistence? If it is stored in any database, then this is a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to test/fix backward compatibility , which is what is remaining.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to handle this would be to detect breakage and recompile from source. Since the IR is not exposed to the user.

string variant_constructor = 2;
string enum_constructor = 3;
}
Expand Down Expand Up @@ -217,3 +217,44 @@ message TupleConstructorArmPattern {
message LiteralArmPattern {
Expr expr = 1;
}

message DynamicParsedFunctionName {
golem.rib.ParsedFunctionSite site = 1;
DynamicParsedFunctionReference function = 2;
}

message DynamicParsedFunctionReference {
oneof function_reference {
golem.rib.FunctionFunctionReference function = 1;
golem.rib.RawResourceConstructorFunctionReference raw_resource_constructor = 2;
golem.rib.RawResourceDropFunctionReference raw_resource_drop = 3;
golem.rib.RawResourceMethodFunctionReference raw_resource_method = 4;
golem.rib.RawResourceStaticMethodFunctionReference raw_resource_static_method = 5;
DynamicIndexedResourceConstructorFunctionReference indexed_resource_constructor = 6;
DynamicIndexedResourceMethodFunctionReference indexed_resource_method = 7;
DynamicIndexedResourceStaticMethodFunctionReference indexed_resource_static_method = 8;
DynamicIndexedResourceDropFunctionReference indexed_resource_drop = 9;
}
}

message DynamicIndexedResourceConstructorFunctionReference {
string resource = 1;
repeated golem.rib.Expr resource_params = 2;
}

message DynamicIndexedResourceMethodFunctionReference {
string resource = 1;
repeated golem.rib.Expr resource_params = 2;
string method = 3;
}

message DynamicIndexedResourceStaticMethodFunctionReference {
string resource = 1;
repeated golem.rib.Expr resource_params = 2;
string method = 3;
}

message DynamicIndexedResourceDropFunctionReference {
string resource = 1;
repeated golem.rib.Expr resource_params = 2;
}
2 changes: 1 addition & 1 deletion golem-api-grpc/proto/golem/rib/function_name.proto
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,4 @@ message IndexedResourceStaticMethodFunctionReference {
message IndexedResourceDropFunctionReference {
string resource = 1;
repeated string resource_params = 2;
}
}
68 changes: 67 additions & 1 deletion golem-api-grpc/proto/golem/rib/ir.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import "wasm/ast/type.proto";

import "wasm/rpc/type_annotated_value.proto";

import "golem/rib/function_name.proto";

message RibIR {
oneof instruction {
wasm.rpc.TypeAnnotatedValue push_lit = 1;
Expand Down Expand Up @@ -39,6 +41,7 @@ message RibIR {
ConcatInstruction concat = 29;
EnumConstructionInstruction enum_construction = 30;
And and = 31;
CreateFunctionNameInstruction create_function_name = 32;
}
}

Expand Down Expand Up @@ -83,7 +86,6 @@ message JumpInstruction {
}

message CallInstruction {
string function_name = 1;
uint64 argument_count = 2;
wasm.ast.Type return_type = 3;
}
Expand All @@ -98,6 +100,12 @@ message EnumConstructionInstruction {
wasm.ast.Type return_type = 2;
}

message CreateFunctionNameInstruction {
golem.rib.ParsedFunctionSite site = 1;
FunctionReferenceType function_reference_details = 2;
}


message EqualTo {}
message GreaterThan {}
message LessThan {}
Expand All @@ -106,3 +114,61 @@ message LessThanOrEqualTo {}
message GetTag {}
message Negate {}
message And {}

message FunctionReferenceType {
oneof type {
Function function = 1;
RawResourceConstructor raw_resource_constructor = 2;
RawResourceDrop raw_resource_drop = 3;
RawResourceMethod raw_resource_method = 4;
RawResourceStaticMethod raw_resource_static_method = 5;
IndexedResourceConstructor indexed_resource_constructor = 6;
IndexedResourceMethod indexed_resource_method = 7;
IndexedResourceStaticMethod indexed_resource_static_method = 8;
IndexedResourceDrop indexed_resource_drop = 9;
}
}

message Function {
string name = 1;
}

message RawResourceConstructor {
string resource_name = 1;
}

message RawResourceDrop {
string resource_name = 1;
}

message RawResourceMethod {
string resource_name = 1;
string method_name = 2;
}

message RawResourceStaticMethod {
string resource_name = 1;
string method_name = 2;
}

message IndexedResourceConstructor {
string resource_name = 1;
uint32 arg_size = 2;
}

message IndexedResourceMethod {
string resource_name = 1;
uint32 arg_size = 2;
string method_name = 3;
}

message IndexedResourceStaticMethod {
string resource_name = 1;
uint32 arg_size = 2;
string method_name = 3;
}

message IndexedResourceDrop {
string resource_name = 1;
uint32 arg_size = 2;
}
10 changes: 5 additions & 5 deletions golem-rib/src/call_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::ParsedFunctionName;
use crate::DynamicParsedFunctionName;
use bincode::{Decode, Encode};
use std::convert::TryFrom;
use std::fmt::Display;

#[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)]
pub enum CallType {
Function(ParsedFunctionName),
Function(DynamicParsedFunctionName),
VariantConstructor(String),
EnumConstructor(String),
}
Expand All @@ -41,9 +41,9 @@ impl TryFrom<golem_api_grpc::proto::golem::rib::InvocationName> for CallType {
) -> Result<Self, Self::Error> {
let invocation = value.name.ok_or("Missing name of invocation")?;
match invocation {
golem_api_grpc::proto::golem::rib::invocation_name::Name::Parsed(name) => {
Ok(CallType::Function(ParsedFunctionName::try_from(name)?))
}
golem_api_grpc::proto::golem::rib::invocation_name::Name::Parsed(name) => Ok(
CallType::Function(DynamicParsedFunctionName::try_from(name)?),
),
golem_api_grpc::proto::golem::rib::invocation_name::Name::VariantConstructor(name) => {
Ok(CallType::VariantConstructor(name))
}
Expand Down
120 changes: 114 additions & 6 deletions golem-rib/src/compiler/byte_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ impl From<RibByteCode> for ProtoRibByteCode {

mod internal {
use crate::compiler::desugar::desugar_pattern_match;
use crate::{AnalysedTypeWithUnit, Expr, InferredType, InstructionId, RibIR};
use crate::{
AnalysedTypeWithUnit, DynamicParsedFunctionReference, Expr, FunctionReferenceType,
InferredType, InstructionId, RibIR,
};
use golem_wasm_ast::analysis::AnalysedType;
use golem_wasm_rpc::protobuf::type_annotated_value::TypeAnnotatedValue;

Expand Down Expand Up @@ -251,11 +254,115 @@ mod internal {
)?)
};

instructions.push(RibIR::InvokeFunction(
parsed_function_name.clone(),
arguments.len(),
function_result_type,
));
instructions
.push(RibIR::InvokeFunction(arguments.len(), function_result_type));

let site = parsed_function_name.site.clone();

match &parsed_function_name.function {
DynamicParsedFunctionReference::Function { function } => instructions
.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::Function(function.clone()),
)),

DynamicParsedFunctionReference::RawResourceConstructor { resource } => {
instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::RawResourceConstructor(resource.clone()),
))
}
DynamicParsedFunctionReference::RawResourceDrop { resource } => {
instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::RawResourceDrop(resource.clone()),
))
}
DynamicParsedFunctionReference::RawResourceMethod {
resource,
method,
} => instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::RawResourceMethod(
resource.clone(),
method.clone(),
),
)),
DynamicParsedFunctionReference::RawResourceStaticMethod {
resource,
method,
} => instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::RawResourceStaticMethod(
resource.clone(),
method.clone(),
),
)),
DynamicParsedFunctionReference::IndexedResourceConstructor {
resource,
resource_params,
} => {
for param in resource_params {
stack.push(ExprState::from_expr(param));
}
instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::IndexedResourceConstructor(
resource.clone(),
resource_params.len(),
),
))
}
DynamicParsedFunctionReference::IndexedResourceMethod {
resource,
resource_params,
method,
} => {
for param in resource_params {
stack.push(ExprState::from_expr(param));
}
instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::IndexedResourceMethod(
resource.clone(),
resource_params.len(),
method.clone(),
),
))
}
DynamicParsedFunctionReference::IndexedResourceStaticMethod {
resource,
resource_params,
method,
} => {
for param in resource_params {
stack.push(ExprState::from_expr(param));
}
instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::IndexedResourceStaticMethod(
resource.clone(),
resource_params.len(),
method.clone(),
),
))
}
DynamicParsedFunctionReference::IndexedResourceDrop {
resource,
resource_params,
} => {
for param in resource_params {
stack.push(ExprState::from_expr(param));
}
instructions.push(RibIR::CreateFunctionName(
site,
FunctionReferenceType::IndexedResourceDrop(
resource.clone(),
resource_params.len(),
),
))
}
}
}

CallType::VariantConstructor(variant_name) => {
Expand Down Expand Up @@ -380,6 +487,7 @@ mod internal {
stack.push(ExprState::from_ir(RibIR::Label(else_ending_id.clone())));
}
}

#[cfg(test)]
mod compiler_tests {
use super::*;
Expand Down
Loading
Loading