Skip to content

Commit 164552c

Browse files
committed
Allow inlinedtree.Build() to spill large lists into separate objects
inlinedtree.Build() can currently only store messages in separate objects. This is problematic, because we also have many cases in which we want to store lists of objects separately. To achieve this, let's add a Marshalable type to model_core. We can use this to encode all the different strategies of how we marshal data. Right now there are only implementations for plain messages and lists of messages. Maybe later we also want to provide an implementation for blobs, so that we can store large Starlark strings/bytes in separate objects. When storing lists externally, we need to emit parent nodes. For this we already have btree.ParentNodeComputer. Let's reuse this here. Provide a btree.MaybeMergeNodes() function that acts as a bridge between inlinedtree and btree. Because inlinedtree.ParentAppender does not allow returning errors, change btree.ParentNodeComputer to also not allow it. I'm not entirely happy with how friendly the resulting API is to use. Let's see if we can improve it at some point in the future. For example, I don't like how we need to pass the capturer, encoder, and parent node computer around separately, even though I don't think you ever want those to differ. We should really address that at some point in the nearby future.
1 parent 5a60076 commit 164552c

28 files changed

Lines changed: 416 additions & 363 deletions

cmd/bonanza_builder/main.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -374,17 +374,12 @@ func (e *builderExecutor) Execute(ctx context.Context, action *model_build_pb.Ac
374374
btree.NewObjectCreatingNodeMerger(
375375
evaluationTreeEncoder,
376376
namespace.ReferenceFormat,
377-
/* parentNodeComputer = */ func(createdObject model_core.Decodable[model_core.CreatedObject[dag.ObjectContentsWalker]], childNodes []*model_evaluation_pb.Evaluation) (model_core.PatchedMessage[*model_evaluation_pb.Evaluation, dag.ObjectContentsWalker], error) {
377+
/* parentNodeComputer = */ func(createdObject model_core.Decodable[model_core.CreatedObject[dag.ObjectContentsWalker]], childNodes []*model_evaluation_pb.Evaluation) model_core.PatchedMessage[*model_evaluation_pb.Evaluation, dag.ObjectContentsWalker] {
378378
var firstKey []byte
379379
switch firstEntry := childNodes[0].Level.(type) {
380380
case *model_evaluation_pb.Evaluation_Leaf_:
381-
flattenedAny, err := model_core.FlattenAny(model_core.NewMessage(firstEntry.Leaf.Key, createdObject.Value.Contents))
382-
if err != nil {
383-
return model_core.PatchedMessage[*model_evaluation_pb.Evaluation, dag.ObjectContentsWalker]{}, err
384-
}
385-
firstKey, err = model_core.MarshalTopLevelMessage(flattenedAny)
386-
if err != nil {
387-
return model_core.PatchedMessage[*model_evaluation_pb.Evaluation, dag.ObjectContentsWalker]{}, err
381+
if flattenedAny, err := model_core.FlattenAny(model_core.NewMessage(firstEntry.Leaf.Key, createdObject.Value.Contents)); err == nil {
382+
firstKey, _ = model_core.MarshalTopLevelMessage(flattenedAny)
388383
}
389384
case *model_evaluation_pb.Evaluation_Parent_:
390385
firstKey = firstEntry.Parent.FirstKey
@@ -398,7 +393,7 @@ func (e *builderExecutor) Execute(ctx context.Context, action *model_build_pb.Ac
398393
},
399394
},
400395
}
401-
}), nil
396+
})
402397
},
403398
),
404399
)
@@ -446,7 +441,7 @@ func (e *builderExecutor) Execute(ctx context.Context, action *model_build_pb.Ac
446441
btree.NewObjectCreatingNodeMerger(
447442
evaluationTreeEncoder,
448443
namespace.ReferenceFormat,
449-
/* parentNodeComputer = */ func(createdObject model_core.Decodable[model_core.CreatedObject[dag.ObjectContentsWalker]], childNodes []*model_evaluation_pb.Dependency) (model_core.PatchedMessage[*model_evaluation_pb.Dependency, dag.ObjectContentsWalker], error) {
444+
/* parentNodeComputer = */ func(createdObject model_core.Decodable[model_core.CreatedObject[dag.ObjectContentsWalker]], childNodes []*model_evaluation_pb.Dependency) model_core.PatchedMessage[*model_evaluation_pb.Dependency, dag.ObjectContentsWalker] {
450445
return model_core.BuildPatchedMessage(func(patcher *model_core.ReferenceMessagePatcher[dag.ObjectContentsWalker]) *model_evaluation_pb.Dependency {
451446
return &model_evaluation_pb.Dependency{
452447
Level: &model_evaluation_pb.Dependency_Parent_{
@@ -455,7 +450,7 @@ func (e *builderExecutor) Execute(ctx context.Context, action *model_build_pb.Ac
455450
},
456451
},
457452
}
458-
}), nil
453+
})
459454
},
460455
),
461456
)
@@ -533,8 +528,8 @@ func (e *builderExecutor) Execute(ctx context.Context, action *model_build_pb.Ac
533528
}
534529
var evaluationsReference *model_core_pb.WeakDecodableReference
535530
if len(evaluations.Message) > 0 {
536-
createdEvaluations, err := model_core.MarshalAndEncodePatchedListMessage(
537-
evaluations,
531+
createdEvaluations, err := model_core.MarshalAndEncode(
532+
model_core.MessageListToMarshalable(evaluations),
538533
namespace.ReferenceFormat,
539534
evaluationTreeEncoder,
540535
)

pkg/bazelclient/commands/build/do_build.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,8 @@ func DoBuild(args *arguments.BuildCommand, workspacePath path.Parser) {
350350
if l := createdRootDirectory.MaximumSymlinkEscapementLevels; l == nil || l.Value != 0 {
351351
logger.Fatal(formatted.Textf("Module %#v contains one or more symbolic links that potentially escape the module's root directory", moduleName.String()))
352352
}
353-
createdObject, err := model_core.MarshalAndEncodePatchedMessage(
354-
createdModuleRootDirectories[i].Message,
353+
createdObject, err := model_core.MarshalAndEncode(
354+
model_core.MessageToMarshalable(createdModuleRootDirectories[i].Message),
355355
referenceFormat,
356356
directoryParameters.GetEncoder(),
357357
)
@@ -392,8 +392,8 @@ func DoBuild(args *arguments.BuildCommand, workspacePath path.Parser) {
392392
logger.Fatal(formatted.Textf("Failed to create build specification encoder: %s", err))
393393
}
394394

395-
createdBuildSpecification, err := model_core.MarshalAndEncodePatchedMessage(
396-
model_core.NewPatchedMessage(&buildSpecification, buildSpecificationPatcher),
395+
createdBuildSpecification, err := model_core.MarshalAndEncode(
396+
model_core.NewPatchedMessage(model_core.NewMessageMarshalable(&buildSpecification), buildSpecificationPatcher),
397397
referenceFormat,
398398
buildSpecificationEncoder,
399399
)

pkg/model/analysis/action_result.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,22 +109,23 @@ func convertDictToEnvironmentVariableList[TMetadata model_core.ReferenceMetadata
109109
commandEncoder model_encoding.BinaryEncoder,
110110
referenceFormat object.ReferenceFormat,
111111
capturer model_core.CreatedObjectCapturer[TMetadata],
112-
) (model_core.PatchedMessage[[]*model_command_pb.EnvironmentVariableList_Element, TMetadata], error) {
112+
) (model_core.PatchedMessage[[]*model_command_pb.EnvironmentVariableList_Element, TMetadata], btree.ParentNodeComputer[*model_command_pb.EnvironmentVariableList_Element, TMetadata], error) {
113+
parentNodeComputer := func(createdObject model_core.Decodable[model_core.CreatedObject[TMetadata]], childNodes []*model_command_pb.EnvironmentVariableList_Element) model_core.PatchedMessage[*model_command_pb.EnvironmentVariableList_Element, TMetadata] {
114+
return model_core.BuildPatchedMessage(func(patcher *model_core.ReferenceMessagePatcher[TMetadata]) *model_command_pb.EnvironmentVariableList_Element {
115+
return &model_command_pb.EnvironmentVariableList_Element{
116+
Level: &model_command_pb.EnvironmentVariableList_Element_Parent{
117+
Parent: patcher.CaptureAndAddDecodableReference(createdObject, capturer),
118+
},
119+
}
120+
})
121+
}
113122
environmentVariablesBuilder := btree.NewSplitProllyBuilder(
114123
1<<16,
115124
1<<18,
116125
btree.NewObjectCreatingNodeMerger(
117126
commandEncoder,
118127
referenceFormat,
119-
/* parentNodeComputer = */ func(createdObject model_core.Decodable[model_core.CreatedObject[TMetadata]], childNodes []*model_command_pb.EnvironmentVariableList_Element) (model_core.PatchedMessage[*model_command_pb.EnvironmentVariableList_Element, TMetadata], error) {
120-
return model_core.BuildPatchedMessage(func(patcher *model_core.ReferenceMessagePatcher[TMetadata]) *model_command_pb.EnvironmentVariableList_Element {
121-
return &model_command_pb.EnvironmentVariableList_Element{
122-
Level: &model_command_pb.EnvironmentVariableList_Element_Parent{
123-
Parent: patcher.CaptureAndAddDecodableReference(createdObject, capturer),
124-
},
125-
}
126-
}), nil
127-
},
128+
parentNodeComputer,
128129
),
129130
)
130131
for _, name := range slices.Sorted(maps.Keys(environment)) {
@@ -138,8 +139,9 @@ func convertDictToEnvironmentVariableList[TMetadata model_core.ReferenceMetadata
138139
},
139140
}),
140141
); err != nil {
141-
return model_core.PatchedMessage[[]*model_command_pb.EnvironmentVariableList_Element, TMetadata]{}, err
142+
return model_core.PatchedMessage[[]*model_command_pb.EnvironmentVariableList_Element, TMetadata]{}, nil, err
142143
}
143144
}
144-
return environmentVariablesBuilder.FinalizeList()
145+
envList, err := environmentVariablesBuilder.FinalizeList()
146+
return envList, parentNodeComputer, err
145147
}

pkg/model/analysis/base_computer_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ func attachMessageObject[TMessage proto.Message](
148148
builder func(childPatcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) TMessage,
149149
) *model_core_pb.DecodableReference {
150150
createdObject := util.Must(
151-
model_core.MarshalAndEncodePatchedMessage(
152-
model_core.BuildPatchedMessage(builder),
151+
model_core.MarshalAndEncode(
152+
model_core.MessageToMarshalable(model_core.BuildPatchedMessage(builder)),
153153
util.Must(object.NewReferenceFormat(object_pb.ReferenceFormat_SHA256_V1)),
154154
model_encoding.NewChainedBinaryEncoder(nil),
155155
),

0 commit comments

Comments
 (0)