Skip to content
This repository has been archived by the owner on Jun 21, 2022. It is now read-only.

fix(analyzer): improve performance #314

Merged
merged 29 commits into from
Dec 23, 2021
Merged

fix(analyzer): improve performance #314

merged 29 commits into from
Dec 23, 2021

Conversation

masahiro331
Copy link
Collaborator

@masahiro331 masahiro331 commented Oct 23, 2021

Fix the following issue

aquasecurity/trivy#1295
aquasecurity/trivy#1284

Description

This pull request modifies the walker implementation to improve the amount of memory with some analyzers.

For Filesystem command case

If the file size is large

  1. Run os.Open for each execution
  2. Also return the clean function and delete the file after everything is done

If the file size is small, read the file only once

For Image command case

When the file size is large

  1. Get the file from tar and write it to a local file only once
  2. Cache the temp file name
  3. Open for each execution.
  4. Delete the file after everything is done

If the file size is small, read the file only once

analyzer/pkg/rpm/rpm.go Show resolved Hide resolved
analyzer/os/amazonlinux/amazonlinux_test.go Outdated Show resolved Hide resolved
analyzer/language/python/packaging/packaging.go Outdated Show resolved Hide resolved
pkginfoInZip, err := a.analyzeEggZip(content)
if err != nil {
return nil, xerrors.Errorf("egg analysis error: %w", err)
}
content = pkginfoInZip
r = bytes.NewReader(pkginfoInZip)
Copy link
Collaborator

Choose a reason for hiding this comment

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

analyzeEggZip should return io.Reader

analyzer/language/analyze_test.go Outdated Show resolved Hide resolved
walker/tar.go Outdated Show resolved Hide resolved
walker/tar.go Outdated Show resolved Hide resolved
walker/walk.go Show resolved Hide resolved
Comment on lines 201 to 206
defer func() {
err := cleaner()
if err != nil {
log.Logger.Warn("Clean temp directory error: %s", err)
}
}()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like this defer will be called many times for the same file and it causes an error possibly. We have to be careful about when we call cleaner().

}
wg.Wait()
Copy link
Collaborator

Choose a reason for hiding this comment

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

What made you add this here? It looks problematic.

@AkselAllas
Copy link

This seems like an important improvement. Any timeline for this?

@knqyf263 knqyf263 changed the title Improve FileOpener with walker package fix(analyzer): improve performance Dec 23, 2021
@knqyf263 knqyf263 merged commit 6726966 into main Dec 23, 2021
@knqyf263 knqyf263 deleted the feat/walk_process branch December 23, 2021 18:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants