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

util/compression: consolidate implementations #5354

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

This was part of some (unfinished) work to change compressions to use a "registration"; this would allow additional compressions to be added through separate packages, and register themselves, without them being part of the util/compression package itself (and because of this, being considered required dependencies).

  • util/compression: move type definition together with implementation
  • util/compression: consolidate Parse implementations
    Similar to some existing functions, use an un-exported package variable to register mappings.
  • util/compression: consolidate FromMediaType implementations
    Similar to some existing functions, use an un-exported package variable to register mappings.

Similar to some existing functions, use an un-exported package variable
to register mappings.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Similar to some existing functions, use an un-exported package variable
to register mappings.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah marked this pull request as ready for review September 24, 2024 20:17
@thaJeztah thaJeztah self-assigned this Sep 24, 2024
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Technically, I think the old version is some orders of magnitude faster(not that it matters in practice), doesn't allocate memory in heap, and doesn't need init() for the package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants