Skip to content

Commit

Permalink
Metadata optimize types
Browse files Browse the repository at this point in the history
Summary:
Metadata logic has an inefficiency where is will generate duplicate metadata for external types (types defined in another Thrift file).
For example:
```
foo.thrift:

struct MyFooStruct {}
premadeMyFooStructType = <Some Metadata for MyFooStruct>

bar.thrift:

include foo.thrift
struct MyBarStruct {
    1: foo.MyFooStruct field;
}
premadeMyFooStructType = <Some Metadata for MyFooStruct>
premadeMyBarStructType = <Some Metadata for MyBarStruct, referencing premadeMyFooStructType>
```

In the above example - you can see that we define `premadeMyFooStructType` twice - in the original package (foo.thrift) and in the package that imports foo.thrift - bar.thrift.

To make things efficient we want this:
```
foo.thrift:

struct MyFooStruct {}
premadeMyFooStructType = <Some Metadata for MyFooStruct>

bar.thrift:

include foo.thrift
struct MyBarStruct {
    1: foo.MyFooStruct field;
}
premadeMyBarStructType = <Some Metadata for MyBarStruct, using foo.GetMetadataType("MyFooStruct")>
```

Reviewed By: leoleovich

Differential Revision: D63490533

fbshipit-source-id: c2eb9c504b3ab732ba76d7416cfbafc568c0e5dc
  • Loading branch information
echistyakov authored and facebook-github-bot committed Oct 1, 2024
1 parent 4d7e521 commit e322850
Show file tree
Hide file tree
Showing 86 changed files with 1,802 additions and 98 deletions.
38 changes: 33 additions & 5 deletions thrift/compiler/generate/t_mstch_go_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -685,8 +685,11 @@ class mstch_go_type : public mstch_type {
{
{"type:go_comparable?", &mstch_go_type::is_go_comparable},
{"type:metadata_primitive?", &mstch_go_type::is_metadata_primitive},
{"type:named?", &mstch_go_type::has_name},
{"type:full_name", &mstch_go_type::full_name},
{"type:metadata_name", &mstch_go_type::metadata_name},
{"type:metadata_thrift_type_getter",
&mstch_go_type::metadata_thrift_type_getter},
});
}

Expand All @@ -697,19 +700,44 @@ class mstch_go_type : public mstch_type {
auto real_type = type_->get_true_type();
return go::is_type_metadata_primitive(real_type);
}
mstch::node has_name() { return !type_->name().empty(); }
mstch::node full_name() { return type_->get_full_name(); }
mstch::node metadata_name() {
mstch::node metadata_name() { return metadata_name_(); }
mstch::node metadata_thrift_type_getter() {
// Program will be null for primitive (base) types.
// They should be treated as being from the current program.
auto is_from_current_program = type_->get_program() == nullptr ||
data_.is_current_program(type_->get_program());

if (is_from_current_program) {
// If the type is from the current program, we can simply use its
// corresponding *ThriftType variable already present in the program.
return metadata_name_();
} else {
// If the type is external, we must retrieve it from its corresponding
// program/package using GetMetadataThriftType helper method.
return fmt::format(
"{}.GetMetadataThriftType(\"{}\")",
data_.get_go_package_alias(type_->program()),
type_->get_full_name());
}
}

private:
go::codegen_data& data_;

std::string metadata_name_() {
return "premadeThriftType_" + sanitized_full_name_();
}
std::string sanitized_full_name_() {
std::string full_name = type_->get_full_name();
boost::replace_all(full_name, " ", "");
boost::replace_all(full_name, ".", "_");
boost::replace_all(full_name, ",", "_");
boost::replace_all(full_name, "<", "_");
boost::replace_all(full_name, ">", "");
return "premadeThriftType_" + full_name;
return full_name;
}

private:
go::codegen_data& data_;
};

class mstch_go_typedef : public mstch_typedef {
Expand Down
14 changes: 14 additions & 0 deletions thrift/compiler/generate/templates/go/metadata.go.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ var (
{{/program:thrift_metadata_types}}
)

var premadeThriftTypesMap = map[string]*{{program:metadata_qualifier}}ThriftType{
{{#program:thrift_metadata_types}}
{{#type:named?}}
"{{type:full_name}}": {{type:metadata_name}},
{{/type:named?}}
{{/program:thrift_metadata_types}}
}

var structMetadatas = []*{{program:metadata_qualifier}}ThriftStruct{
{{#program:structs}}
{{^struct:exception?}}
Expand Down Expand Up @@ -81,6 +89,12 @@ var serviceMetadatas = []*{{program:metadata_qualifier}}ThriftService{
{{/program:services}}
}

// GetMetadataThriftType (INTERNAL USE ONLY).
// Returns metadata ThriftType for a given full type name.
func GetMetadataThriftType(fullName string) *{{program:metadata_qualifier}}ThriftType {
return premadeThriftTypesMap[fullName]
}

// GetThriftMetadata returns complete Thrift metadata for current and imported packages.
func GetThriftMetadata() *{{program:metadata_qualifier}}ThriftMetadata {
allEnums := GetEnumsMetadata()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@
SetId({{field:key}}).
SetName("{{field:name}}").
SetIsOptional({{#field:optional?}}true{{/field:optional?}}{{^field:optional?}}false{{/field:optional?}}).
SetType({{#field:type}}{{type:metadata_name}}{{/field:type}})
SetType({{#field:type}}{{type:metadata_thrift_type_getter}}{{/field:type}})
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
{{program:metadata_qualifier}}NewThriftFunction().
SetName("{{function:name}}").
SetIsOneway({{#function:oneway?}}true{{/function:oneway?}}{{^function:oneway?}}false{{/function:oneway?}}).
SetReturnType({{#function:return_type}}{{type:metadata_name}}{{/function:return_type}}){{#function:args?}}.
SetReturnType({{#function:return_type}}{{type:metadata_thrift_type_getter}}{{/function:return_type}}){{#function:args?}}.
SetArguments(
[]*{{program:metadata_qualifier}}ThriftField{
{{#function:args}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
{{#typedef:defined_kind?}}
{{program:metadata_qualifier}}NewThriftTypedefType().
SetName("{{typedef:scoped_name}}").
SetUnderlyingType({{#type:typedef_type}}{{type:metadata_name}}{{/type:typedef_type}}),
SetUnderlyingType({{#type:typedef_type}}{{type:metadata_thrift_type_getter}}{{/type:typedef_type}}),
{{/typedef:defined_kind?}}
{{^typedef:defined_kind?}}
{{#type:typedef_type}}
Expand Down Expand Up @@ -58,18 +58,18 @@
{{#type:list?}}
{{#type:list_elem_type}}
{{program:metadata_qualifier}}NewThriftListType().
SetValueType({{type:metadata_name}}),
SetValueType({{type:metadata_thrift_type_getter}}),
{{/type:list_elem_type}}
{{/type:list?}}
{{#type:map?}}
{{program:metadata_qualifier}}NewThriftMapType().
SetKeyType({{#type:key_type}}{{type:metadata_name}}{{/type:key_type}}).
SetValueType({{#type:value_type}}{{type:metadata_name}}{{/type:value_type}}),
SetKeyType({{#type:key_type}}{{type:metadata_thrift_type_getter}}{{/type:key_type}}).
SetValueType({{#type:value_type}}{{type:metadata_thrift_type_getter}}{{/type:value_type}}),
{{/type:map?}}
{{#type:set?}}
{{#type:set_elem_type}}
{{program:metadata_qualifier}}NewThriftSetType().
SetValueType({{type:metadata_name}}),
SetValueType({{type:metadata_thrift_type_getter}}),
{{/type:set_elem_type}}
{{/type:set?}}
{{/type:typedef?}}
8 changes: 8 additions & 0 deletions thrift/compiler/lib/go/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ void codegen_data::add_to_thrift_metadata_types(
add_to_thrift_metadata_types(val_type, visited_type_names);
}

// Filter out external types (i.e. types defined in other programs).
// Primitive (base) types should be treated as internal and kept.
// For those types - 'program' will be null, so account for that.
if (type->get_program() != nullptr &&
!is_current_program(type->get_program())) {
return;
}

thrift_metadata_types.push_back(type);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,91 @@ var (
)
)

var premadeThriftTypesMap = map[string]*metadata.ThriftType{
"module.Color": premadeThriftType_module_Color,
"module.ThriftAdaptedEnum": premadeThriftType_module_ThriftAdaptedEnum,
"string": premadeThriftType_string,
"module.MyAnnotation": premadeThriftType_module_MyAnnotation,
"i32": premadeThriftType_i32,
"module.i32_5137": premadeThriftType_module_i32_5137,
"module.SetWithAdapter": premadeThriftType_module_SetWithAdapter,
"module.StringWithAdapter": premadeThriftType_module_StringWithAdapter,
"module.ListWithElemAdapter": premadeThriftType_module_ListWithElemAdapter,
"module.ListWithElemAdapter_withAdapter": premadeThriftType_module_ListWithElemAdapter_withAdapter,
"module.ListWithElemAdapter_withAdapter_2312": premadeThriftType_module_ListWithElemAdapter_withAdapter_2312,
"module.map_string_ListWithElemAdapter_withAdapter_8454": premadeThriftType_module_map_string_ListWithElemAdapter_withAdapter_8454,
"binary": premadeThriftType_binary,
"module.binary_5673": premadeThriftType_module_binary_5673,
"i64": premadeThriftType_i64,
"module.MyI64": premadeThriftType_module_MyI64,
"module.DoubleTypedefI64": premadeThriftType_module_DoubleTypedefI64,
"module.Foo": premadeThriftType_module_Foo,
"module.Baz": premadeThriftType_module_Baz,
"module.Foo_6868": premadeThriftType_module_Foo_6868,
"module.Foo_3943": premadeThriftType_module_Foo_3943,
"module.FooWithAdapter": premadeThriftType_module_FooWithAdapter,
"module.FooWithAdapter_9317": premadeThriftType_module_FooWithAdapter_9317,
"module.Baz_7352": premadeThriftType_module_Baz_7352,
"module.DirectlyAdapted": premadeThriftType_module_DirectlyAdapted,
"module.Bar": premadeThriftType_module_Bar,
"module.IndependentDirectlyAdapted": premadeThriftType_module_IndependentDirectlyAdapted,
"module.StructWithFieldAdapter": premadeThriftType_module_StructWithFieldAdapter,
"module.TerseAdaptedFields": premadeThriftType_module_TerseAdaptedFields,
"module.A": premadeThriftType_module_A,
"module.AdaptedA": premadeThriftType_module_AdaptedA,
"module.B": premadeThriftType_module_B,
"module.Config": premadeThriftType_module_Config,
"module.MyStruct": premadeThriftType_module_MyStruct,
"module.DurationMs": premadeThriftType_module_DurationMs,
"module.IOBuf": premadeThriftType_module_IOBuf,
"module.CustomProtocolType": premadeThriftType_module_CustomProtocolType,
"module.IndirectionString": premadeThriftType_module_IndirectionString,
"bool": premadeThriftType_bool,
"module.AdaptedBool": premadeThriftType_module_AdaptedBool,
"module.AdaptedInteger": premadeThriftType_module_AdaptedInteger,
"module.AdaptTestStruct": premadeThriftType_module_AdaptTestStruct,
"byte": premadeThriftType_byte,
"module.AdaptedByte": premadeThriftType_module_AdaptedByte,
"i16": premadeThriftType_i16,
"module.AdaptedShort": premadeThriftType_module_AdaptedShort,
"module.AdaptedLong": premadeThriftType_module_AdaptedLong,
"double": premadeThriftType_double,
"module.AdaptedDouble": premadeThriftType_module_AdaptedDouble,
"module.AdaptedString": premadeThriftType_module_AdaptedString,
"module.AdaptedEnum": premadeThriftType_module_AdaptedEnum,
"module.DoubleTypedefBool": premadeThriftType_module_DoubleTypedefBool,
"module.AdaptTemplatedTestStruct": premadeThriftType_module_AdaptTemplatedTestStruct,
"module.AdaptTemplatedNestedTestStruct": premadeThriftType_module_AdaptTemplatedNestedTestStruct,
"module.AdaptTestUnion": premadeThriftType_module_AdaptTestUnion,
"module.AdaptedStruct": premadeThriftType_module_AdaptedStruct,
"module.DirectlyAdaptedStruct": premadeThriftType_module_DirectlyAdaptedStruct,
"module.AdaptedTypedef": premadeThriftType_module_AdaptedTypedef,
"module.TypedefOfDirect": premadeThriftType_module_TypedefOfDirect,
"module.StructFieldAdaptedStruct": premadeThriftType_module_StructFieldAdaptedStruct,
"module.CircularStruct": premadeThriftType_module_CircularStruct,
"module.CircularAdaptee": premadeThriftType_module_CircularAdaptee,
"module.AdaptedCircularAdaptee": premadeThriftType_module_AdaptedCircularAdaptee,
"module.DeclaredAfterStruct": premadeThriftType_module_DeclaredAfterStruct,
"module.ReorderedStruct": premadeThriftType_module_ReorderedStruct,
"module.RenamedStruct": premadeThriftType_module_RenamedStruct,
"module.SameNamespaceStruct": premadeThriftType_module_SameNamespaceStruct,
"module.HeapAllocated": premadeThriftType_module_HeapAllocated,
"module.MoveOnly": premadeThriftType_module_MoveOnly,
"module.AlsoMoveOnly": premadeThriftType_module_AlsoMoveOnly,
"module.ApplyAdapter": premadeThriftType_module_ApplyAdapter,
"module.TransitiveAdapted": premadeThriftType_module_TransitiveAdapted,
"module.CountingInt": premadeThriftType_module_CountingInt,
"module.CountingStruct": premadeThriftType_module_CountingStruct,
"module.Person": premadeThriftType_module_Person,
"module.Person2": premadeThriftType_module_Person2,
"module.RenamedStructWithStructAdapterAndFieldAdapter": premadeThriftType_module_RenamedStructWithStructAdapterAndFieldAdapter,
"module.MyI32": premadeThriftType_module_MyI32,
"module.StructWithAdapter": premadeThriftType_module_StructWithAdapter,
"module.UnionWithAdapter": premadeThriftType_module_UnionWithAdapter,
"module.MyI32_4873": premadeThriftType_module_MyI32_4873,
"module.StringWithAdapter_7208": premadeThriftType_module_StringWithAdapter_7208,
}

var structMetadatas = []*metadata.ThriftStruct{
metadata.NewThriftStruct().
SetName("module.MyAnnotation").
Expand Down Expand Up @@ -1173,6 +1258,12 @@ var serviceMetadatas = []*metadata.ThriftService{
),
}

// GetMetadataThriftType (INTERNAL USE ONLY).
// Returns metadata ThriftType for a given full type name.
func GetMetadataThriftType(fullName string) *metadata.ThriftType {
return premadeThriftTypesMap[fullName]
}

// GetThriftMetadata returns complete Thrift metadata for current and imported packages.
func GetThriftMetadata() *metadata.ThriftMetadata {
allEnums := GetEnumsMetadata()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,31 @@ var (
)
)

var premadeThriftTypesMap = map[string]*metadata.ThriftType{
"cpp.RefType": premadeThriftType_cpp_RefType,
"cpp.EnumUnderlyingType": premadeThriftType_cpp_EnumUnderlyingType,
"string": premadeThriftType_string,
"cpp.Type": premadeThriftType_cpp_Type,
"cpp.Ref": premadeThriftType_cpp_Ref,
"cpp.Name": premadeThriftType_cpp_Name,
"bool": premadeThriftType_bool,
"cpp.Lazy": premadeThriftType_cpp_Lazy,
"cpp.DisableLazyChecksum": premadeThriftType_cpp_DisableLazyChecksum,
"cpp.Adapter": premadeThriftType_cpp_Adapter,
"cpp.PackIsset": premadeThriftType_cpp_PackIsset,
"cpp.MinimizePadding": premadeThriftType_cpp_MinimizePadding,
"cpp.ScopedEnumAsUnionType": premadeThriftType_cpp_ScopedEnumAsUnionType,
"cpp.FieldInterceptor": premadeThriftType_cpp_FieldInterceptor,
"cpp.UseOpEncode": premadeThriftType_cpp_UseOpEncode,
"cpp.EnumType": premadeThriftType_cpp_EnumType,
"cpp.Frozen2Exclude": premadeThriftType_cpp_Frozen2Exclude,
"cpp.Frozen2RequiresCompleteContainerParams": premadeThriftType_cpp_Frozen2RequiresCompleteContainerParams,
"cpp.ProcessInEbThreadUnsafe": premadeThriftType_cpp_ProcessInEbThreadUnsafe,
"cpp.RuntimeAnnotation": premadeThriftType_cpp_RuntimeAnnotation,
"cpp.UseCursorSerialization": premadeThriftType_cpp_UseCursorSerialization,
"cpp.GenerateDeprecatedHeaderClientMethods": premadeThriftType_cpp_GenerateDeprecatedHeaderClientMethods,
}

var structMetadatas = []*metadata.ThriftStruct{
metadata.NewThriftStruct().
SetName("cpp.Type").
Expand Down Expand Up @@ -295,6 +320,12 @@ var enumMetadatas = []*metadata.ThriftEnum{
var serviceMetadatas = []*metadata.ThriftService{
}

// GetMetadataThriftType (INTERNAL USE ONLY).
// Returns metadata ThriftType for a given full type name.
func GetMetadataThriftType(fullName string) *metadata.ThriftType {
return premadeThriftTypesMap[fullName]
}

// GetThriftMetadata returns complete Thrift metadata for current and imported packages.
func GetThriftMetadata() *metadata.ThriftMetadata {
allEnums := GetEnumsMetadata()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,20 @@ var (
)
)

var premadeThriftTypesMap = map[string]*metadata.ThriftType{
"string": premadeThriftType_string,
"hack.FieldWrapper": premadeThriftType_hack_FieldWrapper,
"hack.Wrapper": premadeThriftType_hack_Wrapper,
"hack.Adapter": premadeThriftType_hack_Adapter,
"hack.SkipCodegen": premadeThriftType_hack_SkipCodegen,
"hack.Name": premadeThriftType_hack_Name,
"hack.UnionEnumAttributes": premadeThriftType_hack_UnionEnumAttributes,
"hack.StructTrait": premadeThriftType_hack_StructTrait,
"hack.Attributes": premadeThriftType_hack_Attributes,
"hack.StructAsTrait": premadeThriftType_hack_StructAsTrait,
"hack.ModuleInternal": premadeThriftType_hack_ModuleInternal,
}

var structMetadatas = []*metadata.ThriftStruct{
metadata.NewThriftStruct().
SetName("hack.FieldWrapper").
Expand Down Expand Up @@ -197,6 +211,12 @@ var enumMetadatas = []*metadata.ThriftEnum{
var serviceMetadatas = []*metadata.ThriftService{
}

// GetMetadataThriftType (INTERNAL USE ONLY).
// Returns metadata ThriftType for a given full type name.
func GetMetadataThriftType(fullName string) *metadata.ThriftType {
return premadeThriftTypesMap[fullName]
}

// GetThriftMetadata returns complete Thrift metadata for current and imported packages.
func GetThriftMetadata() *metadata.ThriftMetadata {
allEnums := GetEnumsMetadata()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ var (
)
)

var premadeThriftTypesMap = map[string]*metadata.ThriftType{
"python.Py3Hidden": premadeThriftType_python_Py3Hidden,
"string": premadeThriftType_string,
"python.PyDeprecatedHidden": premadeThriftType_python_PyDeprecatedHidden,
"python.Flags": premadeThriftType_python_Flags,
"python.Name": premadeThriftType_python_Name,
"python.Adapter": premadeThriftType_python_Adapter,
"bool": premadeThriftType_bool,
"python.UseCAPI": premadeThriftType_python_UseCAPI,
}

var structMetadatas = []*metadata.ThriftStruct{
metadata.NewThriftStruct().
SetName("python.Py3Hidden").
Expand Down Expand Up @@ -122,6 +133,12 @@ var enumMetadatas = []*metadata.ThriftEnum{
var serviceMetadatas = []*metadata.ThriftService{
}

// GetMetadataThriftType (INTERNAL USE ONLY).
// Returns metadata ThriftType for a given full type name.
func GetMetadataThriftType(fullName string) *metadata.ThriftType {
return premadeThriftTypesMap[fullName]
}

// GetThriftMetadata returns complete Thrift metadata for current and imported packages.
func GetThriftMetadata() *metadata.ThriftMetadata {
allEnums := GetEnumsMetadata()
Expand Down
Loading

0 comments on commit e322850

Please sign in to comment.