Mojom parser: Stop populating the complete_typeset field.
The complete_typeset field is expensive to populate because it requires a depth-first search through the type graph rooted at each top-level interface. We originally thought this would be needed for runtime type information but it is currently not be used at all for that purpose. For now we are going to stop populating the field and add a comment that it is deprecated. We will likely remove it in the future.
R=azani@chromium.org
Review URL: https://codereview.chromium.org/1786543002 .
diff --git a/mojo/public/interfaces/bindings/mojom_types.mojom b/mojo/public/interfaces/bindings/mojom_types.mojom
index bd29dbc..d532d02 100644
--- a/mojo/public/interfaces/bindings/mojom_types.mojom
+++ b/mojo/public/interfaces/bindings/mojom_types.mojom
@@ -463,6 +463,9 @@
// All of the type_keys of the types in the complete type set of the serive.
// Note that this set is not restricted to the types from a single .mojom
// file.
+ // Note(rudominer) Currently this array is not being populated by the Mojom
+ // parser because it is expensive to do so and the value is not being used.
+ // This field is deprecated and may be removed in the future.
array<string> complete_type_set;
};
diff --git a/mojo/public/tools/bindings/mojom_tool/bin/linux64/mojom.sha1 b/mojo/public/tools/bindings/mojom_tool/bin/linux64/mojom.sha1
index 9a3fce9..8b7ba39 100644
--- a/mojo/public/tools/bindings/mojom_tool/bin/linux64/mojom.sha1
+++ b/mojo/public/tools/bindings/mojom_tool/bin/linux64/mojom.sha1
@@ -1 +1 @@
-f130f82e7790b5eaeb221381809f7c7d9d303511
\ No newline at end of file
+293086ec50f4340cdb572d4e506e59eeb49c7fc1
\ No newline at end of file
diff --git a/mojo/public/tools/bindings/mojom_tool/bin/mac64/mojom.sha1 b/mojo/public/tools/bindings/mojom_tool/bin/mac64/mojom.sha1
index 2b9c7cd..0e2a4e7 100644
--- a/mojo/public/tools/bindings/mojom_tool/bin/mac64/mojom.sha1
+++ b/mojo/public/tools/bindings/mojom_tool/bin/mac64/mojom.sha1
@@ -1 +1 @@
-bba12889f5449c3e8b3ea3777f74d8b5a523fb2c
\ No newline at end of file
+4241273401e86b094bf5c6b2e10c8f62dc7dacb7
\ No newline at end of file
diff --git a/mojom/mojom_parser/serialization/serialization.go b/mojom/mojom_parser/serialization/serialization.go
index 9e13b96..0d61160 100644
--- a/mojom/mojom_parser/serialization/serialization.go
+++ b/mojom/mojom_parser/serialization/serialization.go
@@ -28,6 +28,10 @@
// runtime type info.
var emitSerializedRuntimeTypeInfo bool = true
+// By default we do not populate the complete type set of each top-level interface
+// because doing so is expensive and we are not currently using the the data.
+var populateCompleteTypeSet bool = false
+
// Serialize serializes the MojomDescriptor into a binary form that is passed to the
// backend of the compiler in order to invoke the code generators.
// To do this we use Mojo serialization.
@@ -35,20 +39,25 @@
// of the serialized mojom_types.FileGraph.
// This function is not thread safe.
func Serialize(d *mojom.MojomDescriptor, debug bool) (bytes []byte, debugString string, err error) {
- return serialize(d, debug, true, true)
+ return serialize(d, debug, true, true, false)
}
// serialize() is a package-private version of the public method Serialize().
// It is intended for use in tests because it allows setting of the variables
-// emitLineAndColumnNumbers and emitSerializedRuntimeTypeInfo.
-// This function is not thread safe because it accesses the global variables
-// emitLineAndColumnNumbers and emitSerializedRuntimeTypeInfo.
+// emitLineAndColumnNumbers, emitSerializedRuntimeTypeInfo and populateCompleteTypeSet.
+// This function is not thread safe because it sets and accesses these global
+// variables.
func serialize(d *mojom.MojomDescriptor, debug,
- emitLineAndColumnNumbersParam, emitSerializedRuntimeTypeInfoParam bool) (bytes []byte, debugString string, err error) {
+ emitLineAndColumnNumbersParam, emitSerializedRuntimeTypeInfoParam,
+ populateCompleteTypeSetParam bool) (bytes []byte, debugString string, err error) {
+
+ // Save the global state and then set it based on the parameters.
saveEmitLineAndColumnNumbers := emitLineAndColumnNumbers
emitLineAndColumnNumbers = emitLineAndColumnNumbersParam
saveEmitSerializedRuntimeTypeInfo := emitSerializedRuntimeTypeInfo
emitSerializedRuntimeTypeInfo = emitSerializedRuntimeTypeInfoParam
+ savePopulateCompleteTypeSet := populateCompleteTypeSet
+ populateCompleteTypeSet = populateCompleteTypeSetParam
fileGraph := translateDescriptor(d)
if debug {
@@ -59,8 +68,10 @@
fileGraph.Encode(encoder)
bytes, _, err = encoder.Data()
+ // Restore the original values of the global state.
emitLineAndColumnNumbers = saveEmitLineAndColumnNumbers
emitSerializedRuntimeTypeInfo = saveEmitSerializedRuntimeTypeInfo
+ populateCompleteTypeSet = savePopulateCompleteTypeSet
return
}
@@ -243,7 +254,9 @@
if isTopLevel {
serviceTypeInfo := mojom_types.ServiceTypeInfo{}
serviceTypeInfo.TopLevelInterface = intrfc.TypeKey()
- serviceTypeInfo.CompleteTypeSet = intrfc.FindReachableTypes()
+ if populateCompleteTypeSet {
+ serviceTypeInfo.CompleteTypeSet = intrfc.FindReachableTypes()
+ }
typeInfo.ServicesByName[*intrfc.ServiceName] = serviceTypeInfo
}
return
diff --git a/mojom/mojom_parser/serialization/serialization_test.go b/mojom/mojom_parser/serialization/serialization_test.go
index d97459b..277a157 100644
--- a/mojom/mojom_parser/serialization/serialization_test.go
+++ b/mojom/mojom_parser/serialization/serialization_test.go
@@ -1007,14 +1007,14 @@
}
// Serialize
- bytes, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false)
+ bytes, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false, false)
if err != nil {
t.Errorf("Serialization error for %s: %s", c.fileName, err.Error())
continue
}
// Serialize again and check for consistency.
- bytes2, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false)
+ bytes2, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false, false)
if err != nil {
t.Errorf("Serialization error for %s: %s", c.fileName, err.Error())
continue
@@ -1111,7 +1111,7 @@
}
// Serialize
- bytes, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false)
+ bytes, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false, false)
if err != nil {
t.Errorf("Serialization error for %s: %s", c.fileName, err.Error())
continue
@@ -1390,7 +1390,7 @@
}
// Serialize
- bytes, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false)
+ bytes, _, err := serialize(descriptor, false, c.lineAndcolumnNumbers, false, false)
if err != nil {
t.Errorf("Serialization error for case %d: %s", i, err.Error())
continue
@@ -2014,7 +2014,7 @@
}
// Serialize
- bytes, _, err := serialize(descriptor, false, false, true)
+ bytes, _, err := serialize(descriptor, false, false, true, true)
if err != nil {
t.Errorf("Serialization error for case %d: %s", i, err.Error())
continue
@@ -2040,6 +2040,28 @@
if err := compareTwoGoObjects(c.expectedRuntimeTypeInfoB, &runtimeTypeInfoB); err != nil {
t.Errorf("case %d B:\n%s", i, err.Error())
}
+
+ // Test the parameter populateCompleteTypeSet. We set the final
+ // parameter to false.
+ bytes, _, err = serialize(descriptor, false, false, true, false)
+ if err != nil {
+ t.Errorf("Serialization error for case %d: %s", i, err.Error())
+ continue
+ }
+
+ // Deserialize
+ decoder = bindings.NewDecoder(bytes, nil)
+ fileGraph = mojom_files.MojomFileGraph{}
+ fileGraph.Decode(decoder)
+ runtimeTypeInfoA = deserializeRuntimeTypeInfo(*fileGraph.Files[fileNameA].SerializedRuntimeTypeInfo)
+
+ // Check that CompleteTypeSet has not been populated for any service.
+ for name, service := range runtimeTypeInfoA.ServicesByName {
+ length := len(service.CompleteTypeSet)
+ if length != 0 {
+ t.Errorf("len(CompleteTypeSet)=%d for service=%q", length, name)
+ }
+ }
}
}