Skip to content

Comments

Add test for package priority size#10851

Closed
konstin wants to merge 0 commit intomainfrom
konsti/assert-package-priority-size
Closed

Add test for package priority size#10851
konstin wants to merge 0 commit intomainfrom
konsti/assert-package-priority-size

Conversation

@konstin
Copy link
Member

@konstin konstin commented Jan 22, 2025

See also #10833

@konstin konstin added the internal A refactor or improvement that is not user-facing label Jan 22, 2025
@konstin
Copy link
Member Author

konstin commented Jan 22, 2025

This is an optimization that rustc misses, the type below stores the same data and is 16 bytes:

    #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
    pub(crate) enum PubGrubPriority {
        /// The package has no specific priority.
        ///
        /// As such, its priority is based on the order in which the packages were added (FIFO), such
        /// that the first package we visit is prioritized over subsequent packages.
        ///
        /// TODO(charlie): Prefer constrained over unconstrained packages, if they're at the same depth
        /// in the dependency graph.
        Unspecified(Reverse<usize>, u32),

        /// Selected version of this package were often the culprit of rejecting another package, so
        /// it's deprioritized behind `ConflictEarly`. It's still the higher than `Unspecified` to
        /// conflict before selecting unrelated packages.
        ConflictLate(Reverse<usize>, u32),

        /// Selected version of this package were often rejected, so it's prioritized over
        /// `ConflictLate`.
        ConflictEarly(Reverse<usize>, u32),

        /// The version range is constrained to a single version (e.g., with the `==` operator).
        Singleton(Reverse<usize>, u32),

        /// The package was specified via a direct URL.
        ///
        /// N.B.: URLs need to have priority over registry distributions for correctly matching registry
        /// distributions to URLs, see [`PubGrubPackage::from_package`] an
        /// [`ForkUrls`].
        DirectUrl(Reverse<usize>, u32),

        /// The package is the root package.
        Root,
    }

konstin added a commit that referenced this pull request Jan 22, 2025
Prioritization is performance critical in pubgrub.

Closes #10851
konstin added a commit that referenced this pull request Jan 22, 2025
Prioritization is performance critical in pubgrub.

Closes #10851
konstin added a commit that referenced this pull request Jan 23, 2025
Prioritization is performance critical in pubgrub.

Closes #10851
@konstin konstin closed this Feb 19, 2025
@konstin konstin force-pushed the konsti/assert-package-priority-size branch from 8a262b0 to da30cc4 Compare February 19, 2025 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user-facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant