Skip to content

feat: cache parsing results of current dependency files#342

Merged
maxrake merged 2 commits into
mainfrom
cache_current_parsing
Oct 27, 2023
Merged

feat: cache parsing results of current dependency files#342
maxrake merged 2 commits into
mainfrom
cache_current_parsing

Conversation

@maxrake
Copy link
Copy Markdown
Contributor

@maxrake maxrake commented Oct 27, 2023

This change speeds up overall execution time, especially for manifests.
It does this by caching the results of parsing dependency files from the
current (HEAD) location under analysis. This reduces the instances of
parsing the same file from three to two. Previously, it would happen:

  • When filtering files supplied as input
  • When calculating the new dependencies
    • File has changed (or --force-analysis provided) and --all-deps
      not supplied
    • This one is eliminated via caching
  • When submitting the files for Phylum analysis
    • During the PhylumApi.parseLockfile extension API call
    • This one remains since the extension API does not recognize the
      Python cache

There is also large change to rename internal uses of lockfile term.
This was done in a separate commit and includes the following basic
changes:

  • Rename the phylum.ci.lockfile module to phylum.ci.depfile
  • Rename the Lockfile class to Depfile
  • Change internal naming uses of "lockfile" to "depfile"
  • Change text/comment uses of "lockfile" to "dependency file"
  • Keep the instances where Phylum CLI still uses "lockfile" naming
    • Related names are similarly unchanged, to stay consistent
    • These may be changed if/when the CLI changes to "depfile" language
  • Keep the --lockfile argument for the phylum-ci command
    • Changing this would be a breaking change
    • It would need to happen in coordination with documentation updates
    • It may happen separately at a later time

Testing

Changes from this PR have been built in Docker images and hosted on Docker Hub for my account, as the cache_current_parsing tag.

Testing was completed with the private isildurs_bane repo, where it saved ~40s (2m40s --> 2m).
It was also completed with the public deepcausality-rs/deep_causality repo, where it saved ~3s (23s --> 20s).

This change speeds up overall execution time, especially for manifests.
It does this by caching the results of parsing dependency files from the
current (HEAD) location under analysis.
* Rename the `phylum.ci.lockfile` module to `phylum.ci.depfile`
* Rename the `Lockfile` class to `Depfile`
* Change internal naming uses of "lockfile" to "depfile"
* Change text/comment uses of "lockfile" to "dependency file"
* Keep the instances where Phylum CLI still uses "lockfile" naming
  * Related names are similarly unchanged, to stay consistent
  * These may be changed if/when the CLI changes to "depfile" language
@maxrake maxrake self-assigned this Oct 27, 2023
@maxrake maxrake requested a review from a team as a code owner October 27, 2023 02:32
@maxrake maxrake requested a review from cd-work October 27, 2023 02:32
Copy link
Copy Markdown

@cd-work cd-work left a comment

Choose a reason for hiding this comment

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

import cache 😄

@maxrake maxrake merged commit 1ceff86 into main Oct 27, 2023
@maxrake maxrake deleted the cache_current_parsing branch October 27, 2023 21: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.

2 participants