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

Use a SmallString for the Yanked enum #11715

Merged
merged 1 commit into from
Feb 24, 2025
Merged

Use a SmallString for the Yanked enum #11715

merged 1 commit into from
Feb 24, 2025

Conversation

charliermarsh
Copy link
Member

Summary

This is stored on File, which we create extensively. Easy way to reduce size.

@charliermarsh charliermarsh added the performance Potential performance improvement label Feb 22, 2025
Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

Should we use a Option<Box<Yanked>> on File? This should be even smaller (usize) and yanked versions are rare.

@charliermarsh
Copy link
Member Author

We should validate that that’s smaller, since SmallString is also a single word (but Yanked might be two now with discriminant).

@charliermarsh
Copy link
Member Author

I can check and change if necessary before merging; no need for you to spend more time on it.

@konstin
Copy link
Member

konstin commented Feb 24, 2025

I've confirmed that that's the case on x86_64 with a test (below). The background is that even though Arc (and its variant such as ArcStr) are a pointer, and an x86_64 pointer only uses 48 bits of its 64 bits (leaving us 16 bit), rustc only sees a NonNull with a single niche (Null). Yanked needs at least two niches to represent Yanked::Bool, so it doesn't fit and we get padded to two words. In the experiments I made, all instances of enums where one variant contained a pointer and there was more than a single other variant without (like Option<T>), the enum would get padded to two words.

I don't feel strongly about what we do with File here, its 432 vs 424 bytes, this is for context.

    #[test]
    fn yanked_size() {
        assert_eq!(size_of::<Option<Yanked>>(), size_of::<usize>() * 2);
        assert_eq!(size_of::<Option<Box<Yanked>>>(), size_of::<usize>());
    }

    #[test]
    #[cfg(target_pointer_width = "64")]
    fn file_size() {
        assert_eq!(size_of::<Option<crate::File>>(), 432);
        #[derive(Debug, Clone, Deserialize)]
        #[serde(rename_all = "kebab-case")]
        pub struct File2 {
            // PEP 714-renamed field, followed by PEP 691-compliant field, followed by non-PEP 691-compliant
            // alias used by PyPI.
            pub core_metadata: Option<CoreMetadata>,
            pub dist_info_metadata: Option<CoreMetadata>,
            pub data_dist_info_metadata: Option<CoreMetadata>,
            pub filename: String,
            pub hashes: Hashes,
            /// There are a number of invalid specifiers on PyPI, so we first try to parse it into a
            /// [`VersionSpecifiers`] according to spec (PEP 440), then a [`LenientVersionSpecifiers`] with
            /// fixup for some common problems and if this still fails, we skip the file when creating a
            /// version map.
            #[serde(
                default,
                deserialize_with = "crate::simple_json::deserialize_version_specifiers_lenient"
            )]
            pub requires_python: Option<Result<VersionSpecifiers, VersionSpecifiersParseError>>,
            pub size: Option<u64>,
            pub upload_time: Option<Timestamp>,
            pub url: String,
            pub yanked: Option<Box<Yanked>>,
        }
        assert_eq!(size_of::<File2>(), 424);
    }

@charliermarsh
Copy link
Member Author

I'll change separately.

@charliermarsh charliermarsh merged commit 4fc181d into main Feb 24, 2025
92 checks passed
@charliermarsh charliermarsh deleted the charlie/p-6 branch February 24, 2025 19:04
charliermarsh added a commit that referenced this pull request Feb 24, 2025
## Summary

See: #11715.
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Feb 25, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.6.0` -> `0.6.3` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.6.3`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#063)

[Compare Source](astral-sh/uv@0.6.2...0.6.3)

##### Enhancements

-   Allow quotes around command-line options in `requirement.txt files` ([#&#8203;11644](astral-sh/uv#11644))
-   Initialize PEP 723 script in `uv lock --script` ([#&#8203;11717](astral-sh/uv#11717))

##### Configuration

-   Accept multiple `.env` files in `UV_ENV_FILE` ([#&#8203;11665](astral-sh/uv#11665))

##### Performance

-   Reduce overhead in converting resolutions ([#&#8203;11660](astral-sh/uv#11660))
-   Use `SmallString` on `Hashes` ([#&#8203;11756](astral-sh/uv#11756))
-   Use a `Box` for `Yanked` on `File` ([#&#8203;11755](astral-sh/uv#11755))
-   Use a `SmallString` for the `Yanked` enum ([#&#8203;11715](astral-sh/uv#11715))
-   Use boxed slices for hash vector ([#&#8203;11714](astral-sh/uv#11714))
-   Use install concurrency for bytecode compilation too ([#&#8203;11615](astral-sh/uv#11615))

##### Bug fixes

-   Avoid installing duplicate dependencies across conflicting groups ([#&#8203;11653](astral-sh/uv#11653))
-   Check subdirectory existence after cache heal ([#&#8203;11719](astral-sh/uv#11719))
-   Include uppercase platforms for Windows wheels ([#&#8203;11681](astral-sh/uv#11681))
-   Respect existing PEP 723 script settings in `uv add` ([#&#8203;11716](astral-sh/uv#11716))
-   Reuse refined interpreter to create tool environment ([#&#8203;11680](astral-sh/uv#11680))
-   Skip removed directories during bytecode compilation ([#&#8203;11633](astral-sh/uv#11633))
-   Support conflict markers in `uv export` ([#&#8203;11643](astral-sh/uv#11643))
-   Treat lockfile as outdated if (empty) extras are added ([#&#8203;11702](astral-sh/uv#11702))
-   Display path separators as backslashes on Windows ([#&#8203;11667](astral-sh/uv#11667))
-   Display the built file name instead of the canonicalized name in `uv build` ([#&#8203;11593](astral-sh/uv#11593))
-   Fix message when there are no buildable packages ([#&#8203;11722](astral-sh/uv#11722))
-   Re-allow HTTP schemes for Git dependencies ([#&#8203;11687](astral-sh/uv#11687))

##### Documentation

-   Add anchor links to arguments and options in the CLI reference ([#&#8203;11754](astral-sh/uv#11754))
-   Add link to environment marker specification ([#&#8203;11748](astral-sh/uv#11748))
-   Fix missing a closing bracket in the `cache-keys` setting ([#&#8203;11669](astral-sh/uv#11669))
-   Remove the last edited date from documentation pages ([#&#8203;11753](astral-sh/uv#11753))
-   Fix readme typo ([#&#8203;11742](astral-sh/uv#11742))

### [`v0.6.2`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#062)

[Compare Source](astral-sh/uv@0.6.1...0.6.2)

##### Enhancements

-   Add support for constraining build dependencies with `tool.uv.build-constraint-dependencies` ([#&#8203;11585](astral-sh/uv#11585))
-   Sort dependency group keys when adding new group ([#&#8203;11591](astral-sh/uv#11591))

##### Performance

-   Use an `Arc` for index URLs ([#&#8203;11586](astral-sh/uv#11586))

##### Bug fixes

-   Allow use of x86-64 Python on ARM Windows ([#&#8203;11625](astral-sh/uv#11625))
-   Fix an issue where conflict markers could instigate a very large lock file ([#&#8203;11293](astral-sh/uv#11293))
-   Fix duplicate packages with multiple conflicting extras declared ([#&#8203;11513](astral-sh/uv#11513))
-   Respect color settings for log messages ([#&#8203;11604](astral-sh/uv#11604))
-   Eagerly reject unsupported Git schemes ([#&#8203;11514](astral-sh/uv#11514))

##### Documentation

-   Add documentation for specifying Python versions in tool commands ([#&#8203;11598](astral-sh/uv#11598))

### [`v0.6.1`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#061)

[Compare Source](astral-sh/uv@0.6.0...0.6.1)

##### Enhancements

-   Allow users to mark platforms as "required" for wheel coverage ([#&#8203;10067](astral-sh/uv#10067))
-   Warn for builds in non-build and workspace root pyproject.toml ([#&#8203;11394](astral-sh/uv#11394))

##### Bug fixes

-   Add `--all` to `uvx --reinstall` message ([#&#8203;11535](astral-sh/uv#11535))
-   Fallback to `GET` on HTTP 400 when attempting to use range requests for wheel download ([#&#8203;11539](astral-sh/uv#11539))
-   Prefer local variants in preference selection ([#&#8203;11546](astral-sh/uv#11546))
-   Respect verbatim executable name in `uvx` ([#&#8203;11524](astral-sh/uv#11524))

##### Documentation

-   Add documentation for required environments ([#&#8203;11542](astral-sh/uv#11542))
-   Note that `main.py` used to be `hello.py` ([#&#8203;11519](astral-sh/uv#11519))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xNzEuMiIsInVwZGF0ZWRJblZlciI6IjM5LjE3OS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Potential performance improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants