Skip to content

Commit

Permalink
Remove pointers from trailing slice
Browse files Browse the repository at this point in the history
  • Loading branch information
emcfarlane committed Mar 4, 2025
1 parent 90cc4a0 commit cffd97c
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 29 deletions.
53 changes: 26 additions & 27 deletions private/buf/bufgen/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,30 +198,6 @@ func (g *generator) generateCode(
return nil
}

// hashPluginConfigForImage returns a hash of the plugin config for the image.
// This is used to batch plugins that have the same configuration.
// The hash is based on the following properties:
// - ExcludeOptions
// - Strategy
// - RemoteHost
func hashPluginConfigForImage(pluginConfig bufconfig.GeneratePluginConfig) (string, error) {
type imagePluginConfigKey struct {
ExcludeOptions []string
Strategy Strategy
RemoteHost string
}
key := &imagePluginConfigKey{
ExcludeOptions: pluginConfig.ExcludeOptions(),
Strategy: Strategy(pluginConfig.Strategy()),
RemoteHost: pluginConfig.RemoteHost(),
}
bytes, err := json.Marshal(key)
if err != nil {
return "", err
}
return string(bytes), nil
}

func (g *generator) execPlugins(
ctx context.Context,
container app.EnvStdioContainer,
Expand All @@ -235,9 +211,8 @@ func (g *generator) execPlugins(
responses := make([]*pluginpb.CodeGeneratorResponse, len(pluginConfigs))
requiredFeatures := computeRequiredFeatures(image)

// Group the pluginConfigs by their hash. The hash only considers the
// properties ExcludeOptions, Strategy, and RemoteHost.
pluginConfigsForImage, err := slicesext.ToIndexedValuesMapError(pluginConfigs, hashPluginConfigForImage)
// Group the pluginConfigs by the properties ExcludeOptions, Strategy, and RemoteHost.
pluginConfigsForImage, err := slicesext.ToIndexedValuesMapError(pluginConfigs, createPluginConfigKeyForImage)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -514,3 +489,27 @@ type generateOptions struct {
func newGenerateOptions() *generateOptions {
return &generateOptions{}
}

// createPluginConfigKeyForImage returns a string of the plugin config with
// a subset of properties. This is used to batch plugins that have similar
// configuration. The key is based on the following properties:
// - ExcludeOptions
// - Strategy
// - RemoteHost
func createPluginConfigKeyForImage(pluginConfig bufconfig.GeneratePluginConfig) (string, error) {
type pluginConfigKey struct {
ExcludeOptions []string
Strategy Strategy
RemoteHost string
}
key := &pluginConfigKey{
ExcludeOptions: pluginConfig.ExcludeOptions(),
Strategy: Strategy(pluginConfig.Strategy()),
RemoteHost: pluginConfig.RemoteHost(),
}
bytes, err := json.Marshal(key)
if err != nil {
return "", err
}
return string(bytes), nil
}
37 changes: 36 additions & 1 deletion private/bufpkg/bufimage/bufimageutil/bufimageutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func TestPackages(t *testing.T) {
func TestAny(t *testing.T) {
t.Parallel()
runDiffTest(t, "testdata/any", "c1.txtar", WithIncludeTypes("ExtendedAnySyntax"))
runDiffTest(t, "testdata/any", "c2.txtar", WithIncludeTypes("ExtendedAnySyntax_InField")) // c2
runDiffTest(t, "testdata/any", "c2.txtar", WithIncludeTypes("ExtendedAnySyntax_InField"))
runDiffTest(t, "testdata/any", "c3.txtar", WithIncludeTypes("ExtendedAnySyntax_InList"))
runDiffTest(t, "testdata/any", "c4.txtar", WithIncludeTypes("ExtendedAnySyntax_InMap"))
runDiffTest(t, "testdata/any", "d.txtar", WithIncludeTypes("NormalMessageSyntaxValidType"))
Expand Down Expand Up @@ -388,6 +388,41 @@ func TestTypesFromMainModule(t *testing.T) {
assert.ErrorIs(t, err, ErrImageFilterTypeNotFound)
}

func TestMutateInPlace(t *testing.T) {
t.Parallel()
ctx := context.Background()
_, image, err := getImage(ctx, slogtestext.NewLogger(t), "testdata/options", bufimage.WithExcludeSourceCodeInfo())
require.NoError(t, err)

aProtoFile := image.GetFile("a.proto")
aFileDescriptorProto := aProtoFile.FileDescriptorProto()
assert.Len(t, aFileDescriptorProto.MessageType, 2) // Foo, Empty
assert.Len(t, aFileDescriptorProto.EnumType, 1) // FooEnum
assert.Len(t, aFileDescriptorProto.Service, 1)

// Shallow copy
shallowFilteredImage, err := FilterImage(image, WithIncludeTypes("pkg.Foo"))
require.NoError(t, err)

assert.NotSame(t, aFileDescriptorProto, shallowFilteredImage.GetFile("a.proto").FileDescriptorProto())

// Mutate in place
mutateFilteredImage, err := FilterImage(image, WithIncludeTypes("pkg.Foo"), WithMutateInPlace())
require.NoError(t, err)

// Check that the original image was mutated
assert.Same(t, aFileDescriptorProto, mutateFilteredImage.GetFile("a.proto").FileDescriptorProto())
assert.Len(t, aFileDescriptorProto.MessageType, 1) // Foo
if assert.GreaterOrEqual(t, cap(aFileDescriptorProto.MessageType), 2) {
slice := aFileDescriptorProto.MessageType[1:cap(aFileDescriptorProto.MessageType)]
for _, elem := range slice {
assert.Nil(t, elem) // Empty
}
}
assert.Nil(t, aFileDescriptorProto.EnumType)
assert.Nil(t, aFileDescriptorProto.Service)
}

func getImage(ctx context.Context, logger *slog.Logger, testdataDir string, options ...bufimage.BuildImageOption) (storage.ReadWriteBucket, bufimage.Image, error) {
bucket, err := storageos.NewProvider().NewReadWriteBucket(testdataDir)
if err != nil {
Expand Down
10 changes: 9 additions & 1 deletion private/bufpkg/bufimage/bufimageutil/image_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,6 @@ func remapSlice[T any](
if options.mutateInPlace {
newList = list[:toIndex:cap(list)]
} else {
newList = make([]*T, 0, len(list))
newList = append(newList, list[:toIndex]...)
}
}
Expand All @@ -643,6 +642,15 @@ func remapSlice[T any](
sourcePathsRemap.markDeleted(sourcePath)
}
if isDirty {
if len(newList) == 0 {
return nil, true, nil
}
if options.mutateInPlace && newList != nil {
// Zero out the remaining elements.
for i := int(toIndex); i < len(list); i++ {
list[i] = nil
}
}
return newList, true, nil
}
return list, false, nil
Expand Down

0 comments on commit cffd97c

Please sign in to comment.