-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
aws/corehandlers: speed up validation of []byte #539
Conversation
Request validation logic ensures that struct fields have appropriate values. When validating slices and maps, the goal is to find any structs they may contain, since only struct fields have validation rules. Slices of uint8, aka []byte, do not lead to structs and so cannot cause validation to fail. Speed up validation of this (unnamed) type. This is particularly important for APIs such as Kinesis which can contain byte slices for megabytes of data - each byte does not need to be validated individually. The below output of golang.org/x/tools/cmd/benchcmp is for a Kinesis PutRecords request including 100 Records of 10kB each. benchmark old ns/op new ns/op delta BenchmarkValidateAny-8 310052182 216116 -99.93% benchmark old allocs new allocs delta BenchmarkValidateAny-8 3002424 1909 -99.94% benchmark old bytes new bytes delta BenchmarkValidateAny-8 48079408 19274 -99.96%
Looks really good, thanks for taking the time to put together this PR. The only comment I have is a minor tweak to the method used to determine if the value is a // precached byte slice reflect type for comparison
var byteSliceType = reflect.TypeOf([]byte(nil))
// ...
func (v *validator) validateAny(value reflect.Value, path string) {
// ...
case reflect.Slice:
if value.Type() == byteSliceType {
// Skip byte slice types since there is no validation on content
// inside the byte slice.
return
}
// ... This method I think is a little easier to read, and allocates the same memory. Over several benchmark runs it seems to be about 4000 ns/op faster, but that could easily be just my system. |
Yes, that's a lot easier to read. Thanks! |
Looks good thanks for taking the time to create the PR. This will improve the resource usage of the SDK on any API operation with []byte as input parameters. |
aws/corehandlers: speed up validation of []byte
Breaking Change --- * `aws/endpoints`: Several functions and types have been removed * Removes `DecodeModel` and `DecodeModelOptions` from the package ([aws#509](aws/aws-sdk-go-v2#509)) * Remove Region Constants, Partition Constants, and types use for exploring the endpoint data model ([aws#512](aws/aws-sdk-go-v2#512)) * `service/s3/s3crypto`: Package and associated encryption/decryption clients have been removed from the SDK ([aws#511](aws/aws-sdk-go-v2#511)) * `aws/external`: Removes several export constants and types ([aws#508](aws/aws-sdk-go-v2#508)) * No longer exports AWS environment constants used by the external environment configuration loader * `DefaultSharedConfigProfile` is now defined an exported constant * `aws`: `ErrMissingRegion`, `ErrMissingEndpoint`, `ErrStaticCredentialsEmpty` are now concrete error types ([aws#510](aws/aws-sdk-go-v2#510)) Services --- * Synced the V2 SDK with latest AWS service API definitions. SDK Features --- * `aws/signer/v4`: New methods `SignHTTP` and `PresignHTTP` have been added ([aws#519](aws/aws-sdk-go-v2#519)) * `SignHTTP` replaces `Sign`, and usage of `Sign` should be migrated before it's removal at a later date * `PresignHTTP` replaces `Presign`, and usage of `Presign` should be migrated before it's removal at a later date * `DisableRequestBodyOverwrite` and `UnsignedPayload` are now deprecated options and have no effect on `SignHTTP` or `PresignHTTP`. These options will be removed at a later date. * `aws/external`: Add Support for setting a default fallback region and resolving region from EC2 IMDS ([aws#523](aws/aws-sdk-go-v2#523)) * `WithDefaultRegion` helper has been added which can be passed to `LoadDefaultAWSConfig` * This helper can be used to configure a default fallback region in the event a region fails to be resolved from other sources * Support has been added to resolve region using EC2 IMDS when available * The IMDS region will be used if region as not found configured in either the shared config or the process environment. * Fixes [aws#244](aws/aws-sdk-go-v2#244) * Fixes [aws#515](aws/aws-sdk-go-v2#515) SDK Enhancements --- * `service/dynamodb/expression`: Add IsSet helper for ConditionBuilder and KeyConditionBuilder ([aws#494](aws/aws-sdk-go-v2#494)) * Adds a IsSet helper for ConditionBuilder and KeyConditionBuilder to make it easier to determine if the condition builders have any conditions added to them. * Implements [aws#493](aws/aws-sdk-go-v2#493). * `internal/ini`: Normalize Section keys to lowercase ([aws#495](aws/aws-sdk-go-v2#495)) * Update's SDK's ini utility to store all keys as lowercase. This brings the SDK inline with the AWS CLI's behavior. SDK Bugs --- * `internal/sdk`: Fix SDK's UUID utility to handle partial read ([aws#536](aws/aws-sdk-go-v2#536)) * Fixes the SDK's UUID utility to correctly handle partial reads from its crypto rand source. This error was sometimes causing the SDK's InvocationID value to fail to be obtained, due to a partial read from crypto.Rand. * Fix [aws#534](aws/aws-sdk-go-v2#534) * `aws/defaults`: Fix request metadata headers causing signature errors ([aws#536](aws/aws-sdk-go-v2#536)) * Fixes the SDK's adding the request metadata headers in the wrong location within the request handler stack. This created a situation where a request that was retried would sign the new attempt using the old value of the header. The header value would then be changed before sending the request. * Fix [aws#533](aws/aws-sdk-go-v2#533) * Fix [aws#521](aws/aws-sdk-go-v2#521)
Request validation logic ensures that struct fields have appropriate values.
When validating slices and maps, the goal is to find any structs they may
contain, since only struct fields have validation rules. Slices of uint8,
aka []byte, do not lead to structs and so cannot cause validation to fail.
Speed up validation of this (unnamed) type.
This is particularly important for APIs such as Kinesis which can contain
byte slices for megabytes of data - each byte does not need to be validated
individually.
The below output of golang.org/x/tools/cmd/benchcmp is for a Kinesis
PutRecords request including 100 Records of 10kB each.
benchmark old ns/op new ns/op delta
BenchmarkValidateAny-8 310052182 216116 -99.93%
benchmark old allocs new allocs delta
BenchmarkValidateAny-8 3002424 1909 -99.94%
benchmark old bytes new bytes delta
BenchmarkValidateAny-8 48079408 19274 -99.96%