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.

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

perf(misconf): optimize work with context #6968

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

nikpivkin
Copy link
Contributor

@nikpivkin nikpivkin commented Jun 19, 2024

Description

  • The for_each resource attribute is not added to the context because it cannot be accessed, but it can take up a lot of memory.
  • If possible, use the GetAttr method to access cty.Object attributes, instead of converting the object to a map.

Unfortunately there is still the mergeVars function, which cannot be optimised due to some limitations. Because of the impossibility to modify cty.Object directly, it is necessary to convert cty.Object into a map, modify it and create the object from the map again. With a large number of resources, such actions generate a large number of objects and take a decent amount of time due to checks and conversions inside the cty package.

Test config:

locals {
  team_repos = [ for i in range(1000): "repo-${i}"]
  teams = [ for i in range(10): "team-${i}"]
  repositories = merge([for team_id in local.teams : { for repo in local.team_repos : "${team_id}-${repo}" => team_id}]...)
}

resource "aws_ecr_repository" "ecr-repository" {
  for_each = local.repositories

  name                 = each.key
  image_tag_mutability = "IMMUTABLE"
  tags = {
    "Team" : each.value
  }
}

Before:
Memory usage is increasing, scans are not completed in a reasonable amount of time.

After:

/usr/bin/time -al ./trivy conf -q -f json -o report.json /Users/nikita/projects/trivy-test/diss-6958
       29.68 real        40.85 user         0.69 sys
          1221738496  maximum resident set size

Related issues:

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@nikpivkin
Copy link
Contributor Author

@simar7 Generating UUID for a large number of blocks takes an impressive amount of time, as system calls are used. Can we enable pooling, or use a lightweight way to generate id for blocks? The probability of collision is low.

@nikpivkin nikpivkin marked this pull request as ready for review June 19, 2024 13:32
@nikpivkin nikpivkin requested a review from simar7 as a code owner June 19, 2024 13:32
Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>
@simar7
Copy link
Member

simar7 commented Jun 22, 2024

@simar7 Generating UUID for a large number of blocks takes an impressive amount of time, as system calls are used. Can we enable pooling, or use a lightweight way to generate id for blocks? The probability of collision is low.

It makes sense but how much of an improvement is it?

@nikpivkin
Copy link
Contributor Author

nikpivkin commented Aug 7, 2024

@simar7 I think improving ID generation can be done later, if it does become an issue.

@simar7 simar7 closed this Aug 8, 2024
@simar7 simar7 reopened this Aug 8, 2024
@simar7 simar7 enabled auto-merge August 8, 2024 21:52
@simar7 simar7 added this pull request to the merge queue Aug 8, 2024
Merged via the queue into aquasecurity:main with commit 2b6d8d9 Aug 8, 2024
22 checks passed
@aqua-bot aqua-bot mentioned this pull request Aug 8, 2024
@nikpivkin nikpivkin deleted the tf-ctx branch August 13, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

perf(misconf): High memory usage (> 10 GB) on some repos
2 participants