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

Reduce size of manifests #36

Closed
taiki-e opened this issue Dec 24, 2022 · 4 comments
Closed

Reduce size of manifests #36

taiki-e opened this issue Dec 24, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@taiki-e
Copy link
Owner

taiki-e commented Dec 24, 2022

Since #27, we manage tool info as JSON manifests.

However, the manifest size is currently not very optimized.

From some quick tests, it does not appear that the manifest size is affecting the download time of the action itself at this time, but this could become an issue as more tools are supported.

  • As said in Support just and dprint #34 (comment), reducing supported versions doesn't work.
    • That said, it may make sense to drop support for very old versions. For example, I don't think someone to be using a version that is 4 years older than when this action was initially released.
  • Currently, we use a full copy of the corresponding version's manifest for the version that omitted the patch/minor version (and @latest). It appears to make sense to replace this with a reference to the corresponding version.
@taiki-e taiki-e added the enhancement New feature or request label Dec 24, 2022
@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 24, 2022

Some ideas for reducing size:

  • For many crates (cargo-binstall, cargo-nextest, etc), the url of the download artifacts can be generated from a template. That will not only reduce the size of manifests, but also makes updating manifests easier.
  • We can convert the json files to compact json (without newline, indentation, etc) before publishing this repository as an action
  • We can compress the the action when releasing it
  • We can offload the task to cargo-binstall, which does a great job of auto discovering pre-built artifacts and can fallback to quickinstall.

@taiki-e
Copy link
Owner Author

taiki-e commented Dec 25, 2022

Thanks for your ideas.

  • For many crates (cargo-binstall, cargo-nextest, etc), the url of the download artifacts can be generated from a template. That will not only reduce the size of manifests, but also makes updating manifests easier.

We already use the template in the base manifest, so leaving it in the manifest might work well.

  • We can convert the json files to compact json (without newline, indentation, etc) before publishing this repository as an action
  • We can compress the the action when releasing it

Interesting. I'm not sure what the latter means exactly, but currently releases other than the v<major_version> branch add commits before release, so compacting manifests and removing unneeded files as a part it that process might work well.

  • We can offload the task to cargo-binstall, which does a great job of auto discovering pre-built artifacts and can fallback to quickinstall.

Given that we support packages other than Rust, and our manifest generator also does things like getting file hashes, etc., I don't see much advantage from a view of maintenance cost in doing this for tools we officially support.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 25, 2022

Thanks for your ideas.

You are welcome.

  • For many crates (cargo-binstall, cargo-nextest, etc), the url of the download artifacts can be generated from a template. That will not only reduce the size of manifests, but also makes updating manifests easier.

We already use the template in the base manifest, so leaving it in the manifest might work well.

I notice that other softwares, such as cargo-nextest, also shares the same pattern, so I think this alone would significantly reduce the size.

  • We can convert the json files to compact json (without newline, indentation, etc) before publishing this repository as an action
  • We can compress the the action when releasing it

Interesting. I'm not sure what the latter means exactly, but currently releases other than the v<major_version> branch add commits before release, so compacting manifests and removing unneeded files as a part it that process might work well.

For the "compress" part, I mean that we could compress these json files and the scripts with something like zstd, which has very fast decompression speed and I remember its decompression speed is not related to the options passed to compression, that means we might be able to use highest level.

Then we can include a script that extracts this and runs the main.sh

  • We can offload the task to cargo-binstall, which does a great job of auto discovering pre-built artifacts and can fallback to quickinstall.

Given that we support packages other than Rust, and our manifest generator also does things like getting file hashes, etc., I don't see much advantage from a view of maintenance cost in doing this for tools we officially support.

That's true.

@taiki-e
Copy link
Owner Author

taiki-e commented Dec 26, 2022

Implemented the following three patches:

  • Only serialize version if key != version (c11168b) +28 -615
    Addresses this todo comment.
  • Use templates for tools that have consistent pattern in all releases (2be5cc5) +1,444 -4,551
    Implements NobodyXu's idea.
  • Make omitted version just reference to corresponding version (12505be) +271 -2,630
    Implements this idea.

I think this is fine for now.

@taiki-e taiki-e closed this as completed Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants