Skip to content
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.

Sign up for GitHub

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

Jump to bottom

internal/tags: Used shared memory allocation for shared tag schemas #32250

Merged
merged 1 commit into from
Jun 28, 2023
Merged

internal/tags: Used shared memory allocation for shared tag schemas #32250

merged 1 commit into from
Jun 28, 2023

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Jun 27, 2023

Description

Rather than allocating memory for each shared tag schema, reference the same schema data. Each schema.Schema struct allocation is ~304 bytes, which adds up when multiplied times a few hundred resources using it. Not a big reduction by any means, but seems like a quick way to reduce some unnecessary memory usage.

Updated the two code locations which were directly mutating the result of shared tag schemas.

References

Reference: #31722

Output from Acceptance Testing

$ make testacc TESTS=TestAccXXX PKG=ec2

...

@bflad
internal/tags: Used shared memory allocation for shared tag schemas
ead2e7d 
Reference: hashicorp#31722

Rather than allocating memory for each shared tag schema, reference the same schema data. Each `schema.Schema` struct allocation is ~304 bytes, which adds up when multiplied times a few hundred resources using it. Not a big reduction by any means, but seems like a quick way to reduce some unnecessary memory usage.

Updated the two code locations which were directly mutating the result of shared tag schemas.
@github-actions
Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added size/S Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. tags Pertains to resource tagging. needs-triage Waiting for first response or review from a maintainer. labels Jun 27, 2023
@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Jun 28, 2023
@ewbankkit ewbankkit self-assigned this Jun 28, 2023
ewbankkit
ewbankkit approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

% make sane ACCTEST_PARALLELISM=2
==> Sane Check (48 tests of Top 30 resources)
==> Like 'sanity' except full output and stops soon after 1st error
==> NOTE: NOT an exhaustive set of tests! Finds big problems only.
=== RUN   TestAccIAMInstanceProfile_basic
=== PAUSE TestAccIAMInstanceProfile_basic
=== RUN   TestAccIAMInstanceProfile_tags
=== PAUSE TestAccIAMInstanceProfile_tags
=== RUN   TestAccIAMPolicyDocumentDataSource_basic
=== PAUSE TestAccIAMPolicyDocumentDataSource_basic
=== RUN   TestAccIAMPolicyDocumentDataSource_sourceConflicting
=== PAUSE TestAccIAMPolicyDocumentDataSource_sourceConflicting
=== RUN   TestAccIAMPolicy_basic
=== PAUSE TestAccIAMPolicy_basic
=== RUN   TestAccIAMPolicy_tags
=== PAUSE TestAccIAMPolicy_tags
=== RUN   TestAccIAMPolicy_policy
=== PAUSE TestAccIAMPolicy_policy
=== RUN   TestAccIAMRolePolicyAttachment_basic
=== PAUSE TestAccIAMRolePolicyAttachment_basic
=== RUN   TestAccIAMRolePolicyAttachment_disappears
=== PAUSE TestAccIAMRolePolicyAttachment_disappears
=== RUN   TestAccIAMRolePolicyAttachment_Disappears_role
=== PAUSE TestAccIAMRolePolicyAttachment_Disappears_role
=== RUN   TestAccIAMRolePolicy_basic
=== PAUSE TestAccIAMRolePolicy_basic
=== RUN   TestAccIAMRolePolicy_unknownsInPolicy
=== PAUSE TestAccIAMRolePolicy_unknownsInPolicy
=== RUN   TestAccIAMRole_basic
=== PAUSE TestAccIAMRole_basic
=== RUN   TestAccIAMRole_namePrefix
=== PAUSE TestAccIAMRole_namePrefix
=== RUN   TestAccIAMRole_disappears
=== PAUSE TestAccIAMRole_disappears
=== RUN   TestAccIAMRole_InlinePolicy_basic
=== PAUSE TestAccIAMRole_InlinePolicy_basic
=== CONT  TestAccIAMInstanceProfile_basic
=== CONT  TestAccIAMRolePolicyAttachment_disappears
--- PASS: TestAccIAMRolePolicyAttachment_disappears (19.74s)
=== CONT  TestAccIAMPolicy_basic
--- PASS: TestAccIAMInstanceProfile_basic (23.40s)
=== CONT  TestAccIAMRolePolicyAttachment_basic
--- PASS: TestAccIAMPolicy_basic (21.78s)
=== CONT  TestAccIAMPolicy_policy
--- PASS: TestAccIAMRolePolicyAttachment_basic (38.72s)
=== CONT  TestAccIAMPolicy_tags
--- PASS: TestAccIAMPolicy_policy (39.49s)
=== CONT  TestAccIAMRole_basic
--- PASS: TestAccIAMRole_basic (22.92s)
=== CONT  TestAccIAMRole_InlinePolicy_basic
--- PASS: TestAccIAMPolicy_tags (53.07s)
=== CONT  TestAccIAMRole_disappears
--- PASS: TestAccIAMRole_disappears (17.47s)
=== CONT  TestAccIAMRolePolicy_basic
--- PASS: TestAccIAMRole_InlinePolicy_basic (52.12s)
=== CONT  TestAccIAMRolePolicy_unknownsInPolicy
--- PASS: TestAccIAMRolePolicy_basic (36.04s)
=== CONT  TestAccIAMRolePolicyAttachment_Disappears_role
--- PASS: TestAccIAMRolePolicy_unknownsInPolicy (22.16s)
=== CONT  TestAccIAMPolicyDocumentDataSource_basic
--- PASS: TestAccIAMRolePolicyAttachment_Disappears_role (16.93s)
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceConflicting
--- PASS: TestAccIAMPolicyDocumentDataSource_basic (14.43s)
=== CONT  TestAccIAMRole_namePrefix
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceConflicting (14.35s)
=== CONT  TestAccIAMInstanceProfile_tags
--- PASS: TestAccIAMRole_namePrefix (21.56s)
--- PASS: TestAccIAMInstanceProfile_tags (49.38s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/iam	255.505s
=== RUN   TestAccLogsGroup_basic
=== PAUSE TestAccLogsGroup_basic
=== RUN   TestAccLogsGroup_multiple
=== PAUSE TestAccLogsGroup_multiple
=== CONT  TestAccLogsGroup_basic
=== CONT  TestAccLogsGroup_multiple
--- PASS: TestAccLogsGroup_multiple (45.45s)
--- PASS: TestAccLogsGroup_basic (53.36s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/logs	60.517s
=== RUN   TestAccVPCDataSource_basic
=== PAUSE TestAccVPCDataSource_basic
=== RUN   TestAccVPCRouteTableAssociation_Subnet_basic
=== PAUSE TestAccVPCRouteTableAssociation_Subnet_basic
=== RUN   TestAccVPCRouteTable_basic
=== PAUSE TestAccVPCRouteTable_basic
=== RUN   TestAccVPCSecurityGroupRule_race
=== PAUSE TestAccVPCSecurityGroupRule_race
=== RUN   TestAccVPCSecurityGroupRule_protocolChange
=== PAUSE TestAccVPCSecurityGroupRule_protocolChange
=== RUN   TestAccVPCSecurityGroup_basic
=== PAUSE TestAccVPCSecurityGroup_basic
=== RUN   TestAccVPCSecurityGroup_ipRangesWithSameRules
=== PAUSE TestAccVPCSecurityGroup_ipRangesWithSameRules
=== RUN   TestAccVPCSecurityGroup_vpcAllEgress
=== PAUSE TestAccVPCSecurityGroup_vpcAllEgress
=== RUN   TestAccVPCSubnet_basic
=== PAUSE TestAccVPCSubnet_basic
=== RUN   TestAccVPC_tenancy
=== PAUSE TestAccVPC_tenancy
=== CONT  TestAccVPCDataSource_basic
=== CONT  TestAccVPC_tenancy
--- PASS: TestAccVPCDataSource_basic (55.21s)
=== CONT  TestAccVPCSubnet_basic
--- PASS: TestAccVPCSubnet_basic (28.81s)
=== CONT  TestAccVPCSecurityGroup_vpcAllEgress
--- PASS: TestAccVPC_tenancy (91.77s)
=== CONT  TestAccVPCSecurityGroup_ipRangesWithSameRules
--- PASS: TestAccVPCSecurityGroup_vpcAllEgress (29.30s)
=== CONT  TestAccVPCSecurityGroup_basic
--- PASS: TestAccVPCSecurityGroup_ipRangesWithSameRules (31.70s)
=== CONT  TestAccVPCSecurityGroupRule_protocolChange
--- PASS: TestAccVPCSecurityGroup_basic (28.89s)
=== CONT  TestAccVPCSecurityGroupRule_race
--- PASS: TestAccVPCSecurityGroupRule_protocolChange (67.80s)
=== CONT  TestAccVPCRouteTable_basic
--- PASS: TestAccVPCRouteTable_basic (26.14s)
=== CONT  TestAccVPCRouteTableAssociation_Subnet_basic
--- PASS: TestAccVPCRouteTableAssociation_Subnet_basic (49.96s)
--- PASS: TestAccVPCSecurityGroupRule_race (150.24s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/ec2	313.964s
=== RUN   TestAccECSService_basic
=== PAUSE TestAccECSService_basic
=== RUN   TestAccECSService_basicImport
=== PAUSE TestAccECSService_basicImport
=== RUN   TestAccECSTaskDefinition_basic
=== PAUSE TestAccECSTaskDefinition_basic
=== CONT  TestAccECSService_basic
=== CONT  TestAccECSTaskDefinition_basic
--- PASS: TestAccECSTaskDefinition_basic (72.53s)
=== CONT  TestAccECSService_basicImport
--- PASS: TestAccECSService_basic (121.79s)
--- PASS: TestAccECSService_basicImport (206.34s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/ecs	286.773s
=== RUN   TestAccELBV2TargetGroup_basic
=== PAUSE TestAccELBV2TargetGroup_basic
=== CONT  TestAccELBV2TargetGroup_basic
--- PASS: TestAccELBV2TargetGroup_basic (46.97s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/elbv2	60.200s
=== RUN   TestAccKMSKey_basic
=== PAUSE TestAccKMSKey_basic
=== CONT  TestAccKMSKey_basic
--- PASS: TestAccKMSKey_basic (42.84s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/kms	71.342s
=== RUN   TestAccLambdaFunction_basic
=== PAUSE TestAccLambdaFunction_basic
=== RUN   TestAccLambdaPermission_basic
=== PAUSE TestAccLambdaPermission_basic
=== CONT  TestAccLambdaFunction_basic
=== CONT  TestAccLambdaPermission_basic
--- PASS: TestAccLambdaPermission_basic (78.95s)
--- PASS: TestAccLambdaFunction_basic (86.26s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/lambda	91.992s
=== RUN   TestAccMetaPartitionDataSource_basic
=== PAUSE TestAccMetaPartitionDataSource_basic
=== RUN   TestAccMetaRegionDataSource_basic
=== PAUSE TestAccMetaRegionDataSource_basic
=== RUN   TestAccMetaRegionDataSource_endpoint
=== PAUSE TestAccMetaRegionDataSource_endpoint
=== RUN   TestAccMetaRegionDataSource_endpointAndName
=== PAUSE TestAccMetaRegionDataSource_endpointAndName
=== CONT  TestAccMetaPartitionDataSource_basic
=== CONT  TestAccMetaRegionDataSource_endpoint
--- PASS: TestAccMetaPartitionDataSource_basic (43.24s)
=== CONT  TestAccMetaRegionDataSource_basic
--- PASS: TestAccMetaRegionDataSource_endpoint (44.52s)
=== CONT  TestAccMetaRegionDataSource_endpointAndName
--- PASS: TestAccMetaRegionDataSource_basic (34.54s)
--- PASS: TestAccMetaRegionDataSource_endpointAndName (36.35s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/meta	89.989s
=== RUN   TestAccRoute53Record_basic
=== PAUSE TestAccRoute53Record_basic
=== RUN   TestAccRoute53Record_Latency_basic
=== PAUSE TestAccRoute53Record_Latency_basic
=== RUN   TestAccRoute53ZoneDataSource_name
=== PAUSE TestAccRoute53ZoneDataSource_name
=== CONT  TestAccRoute53Record_basic
=== CONT  TestAccRoute53ZoneDataSource_name
--- PASS: TestAccRoute53ZoneDataSource_name (83.39s)
=== CONT  TestAccRoute53Record_Latency_basic
--- PASS: TestAccRoute53Record_basic (151.51s)
--- PASS: TestAccRoute53Record_Latency_basic (164.26s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/route53	259.637s
=== RUN   TestAccS3BucketACL_updateACL
=== PAUSE TestAccS3BucketACL_updateACL
=== RUN   TestAccS3BucketPolicy_basic
=== PAUSE TestAccS3BucketPolicy_basic
=== RUN   TestAccS3BucketPublicAccessBlock_basic
=== PAUSE TestAccS3BucketPublicAccessBlock_basic
=== RUN   TestAccS3Bucket_Basic_basic
=== PAUSE TestAccS3Bucket_Basic_basic
=== RUN   TestAccS3Bucket_Security_corsUpdate
=== PAUSE TestAccS3Bucket_Security_corsUpdate
=== CONT  TestAccS3BucketACL_updateACL
=== CONT  TestAccS3Bucket_Basic_basic
--- PASS: TestAccS3Bucket_Basic_basic (57.75s)
=== CONT  TestAccS3Bucket_Security_corsUpdate
--- PASS: TestAccS3BucketACL_updateACL (85.64s)
=== CONT  TestAccS3BucketPublicAccessBlock_basic
--- PASS: TestAccS3BucketPublicAccessBlock_basic (28.58s)
=== CONT  TestAccS3BucketPolicy_basic
--- PASS: TestAccS3Bucket_Security_corsUpdate (57.11s)
--- PASS: TestAccS3BucketPolicy_basic (27.15s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	159.015s
=== RUN   TestAccSecretsManagerSecret_basic
=== PAUSE TestAccSecretsManagerSecret_basic
=== RUN   TestAccSecretsManagerSecret_basicReplica
=== PAUSE TestAccSecretsManagerSecret_basicReplica
=== CONT  TestAccSecretsManagerSecret_basic
=== CONT  TestAccSecretsManagerSecret_basicReplica
--- PASS: TestAccSecretsManagerSecret_basic (47.76s)
--- PASS: TestAccSecretsManagerSecret_basicReplica (67.45s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/secretsmanager	92.468s
=== RUN   TestAccSTSCallerIdentityDataSource_basic
=== PAUSE TestAccSTSCallerIdentityDataSource_basic
=== CONT  TestAccSTSCallerIdentityDataSource_basic
--- PASS: TestAccSTSCallerIdentityDataSource_basic (32.96s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/sts	64.592s

@ewbankkit
Copy link
Contributor

@bflad Thanks for the contribution 🎉 👏.

@ewbankkit ewbankkit merged commit 527319e into hashicorp:main Jun 28, 2023
49 checks passed
@github-actions github-actions bot added this to the v5.6.0 milestone Jun 28, 2023
@bflad bflad deleted the bflad/shared-tag-schema branch June 28, 2023 13:48
This was referenced Jun 28, 2023
Merged
Merged
@github-actions
Copy link

This functionality has been released in v5.6.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ewbankkit ewbankkit ewbankkit approved these changes

Assignees

@ewbankkit ewbankkit

Labels
service/ec2 Issues and PRs that pertain to the ec2 service. size/S Managed by automation to categorize the size of a PR. tags Pertains to resource tagging.
Projects
None yet
Milestone
v5.6.0
Development

Successfully merging this pull request may close these issues.
2 participants
@bflad @ewbankkit