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

Add interning to Bytes, reducing the need for allocations when slices are small. #231

Merged
merged 6 commits into from
Jul 29, 2024

Conversation

p-avital
Copy link
Contributor

The last bit of the memory layout is the determinant for interning:

  • If it's unset, the layout is as expressed from the struct definition, and the pointer is guaranteed to be a valid reference to a vtable. No legal reference to a vtable can set this bit, as it would infringe on alignment constraints.
  • If the bit is set, the layout is interned: the last byte (shifted by 1) is used as the length of the slice, and the slice starts at the first byte of the memory layout.

Interning lets us have cheap copies and lifetime upgrades for small slices (up to 39 bytes on 64bit archs), as copying them no longer requires an allocation if their original storage doesn't support reference counting or lifetime extension.

This PR also:

  • Removes From for non-static slices, so that From and from_static are now equivalent, and from_slice is now the constructor from borrowed slices. The reasoning behind this is to make the more wasteful variant less ergonomic
  • Adds constructors From &'static [u8;N] and &'static str to reduce boilerplate.

Copy link
Collaborator

@danielhenrymantilla danielhenrymantilla left a comment

Choose a reason for hiding this comment

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

LGTM ✅

  • Naming nit: interning is a term usually referred to using some arena or whatnot to "cache" common slices/strings, etc.. What would you think of using inlining instead? (and mention "small vec optimization" in the docs alongside it)

src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@danielhenrymantilla danielhenrymantilla left a comment

Choose a reason for hiding this comment

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

⚠️

src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
@p-avital
Copy link
Contributor Author

Thanks for catching that endianness issue! Your reviews are always awesome!

I ignored some of the comments that clashed with allowing certain functions to be const :)

src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Outdated Show resolved Hide resolved
src/bytes.rs Show resolved Hide resolved
@danielhenrymantilla danielhenrymantilla merged commit 2485abe into master Jul 29, 2024
206 checks passed
@danielhenrymantilla danielhenrymantilla deleted the better-bytes branch July 29, 2024 16:15
haixuanTao referenced this pull request in dora-rs/dora Aug 6, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [clap](https://togithub.com/clap-rs/clap) | dependencies | patch |
`4.5.11` -> `4.5.13` |
| [dunce](https://lib.rs/crates/dunce)
([source](https://gitlab.com/kornelski/dunce)) | dev-dependencies |
patch | `1.0.4` -> `1.0.5` |
| [regex](https://togithub.com/rust-lang/regex) | dependencies | patch |
`1.10.5` -> `1.10.6` |
| [ros2-client](https://atostek.com/en/products/rustdds/)
([source](https://togithub.com/jhelovuo/ros2-client)) | dependencies |
patch | `0.7.2` -> `0.7.3` |
| [rustdds](https://atostek.com/en/products/rustdds/)
([source](https://togithub.com/jhelovuo/RustDDS)) | dependencies | patch
| `0.10.1` -> `0.10.3` |
| [safer-ffi](https://togithub.com/getditto/safer_ffi) | dependencies |
patch | `0.1.9` -> `0.1.12` |
| [serde_json](https://togithub.com/serde-rs/json) | dependencies |
patch | `1.0.121` -> `1.0.122` |

---

### Release Notes

<details>
<summary>clap-rs/clap (clap)</summary>

###
[`v4.5.13`](https://togithub.com/clap-rs/clap/blob/HEAD/CHANGELOG.md#4513---2024-07-31)

[Compare
Source](https://togithub.com/clap-rs/clap/compare/v4.5.12...v4.5.13)

##### Fixes

- *(derive)* Improve error message when `#[flatten]`ing an optional
`#[group(skip)]`
-   *(help)* Properly wrap long subcommand descriptions in help

###
[`v4.5.12`](https://togithub.com/clap-rs/clap/blob/HEAD/CHANGELOG.md#4512---2024-07-31)

[Compare
Source](https://togithub.com/clap-rs/clap/compare/v4.5.11...v4.5.12)

</details>

<details>
<summary>kornelski/dunce (dunce)</summary>

###
[`v1.0.5`](https://gitlab.com/kornelski/dunce/compare/v1.0.4...v1.0.5)

[Compare
Source](https://gitlab.com/kornelski/dunce/compare/v1.0.4...v1.0.5)

</details>

<details>
<summary>rust-lang/regex (regex)</summary>

###
[`v1.10.6`](https://togithub.com/rust-lang/regex/blob/HEAD/CHANGELOG.md#1106-2024-08-02)

[Compare
Source](https://togithub.com/rust-lang/regex/compare/1.10.5...1.10.6)

\===================
This is a new patch release with a fix for the `unstable` crate feature
that
enables `std::str::Pattern` trait integration.

Bug fixes:

-   [BUG #&#8203;1219](https://togithub.com/rust-lang/regex/pull/1219):
Fix the `Pattern` trait implementation as a result of nightly API
breakage.

</details>

<details>
<summary>jhelovuo/ros2-client (ros2-client)</summary>

###
[`v0.7.3`](https://togithub.com/jhelovuo/ros2-client/compare/0.7.2...0.7.3)

[Compare
Source](https://togithub.com/jhelovuo/ros2-client/compare/0.7.2...0.7.3)

</details>

<details>
<summary>jhelovuo/RustDDS (rustdds)</summary>

###
[`v0.10.3`](https://togithub.com/jhelovuo/RustDDS/compare/0.10.2...0.10.3)

[Compare
Source](https://togithub.com/jhelovuo/RustDDS/compare/0.10.2...0.10.3)

###
[`v0.10.2`](https://togithub.com/jhelovuo/RustDDS/compare/0.10.1...0.10.2)

[Compare
Source](https://togithub.com/jhelovuo/RustDDS/compare/0.10.1...0.10.2)

</details>

<details>
<summary>getditto/safer_ffi (safer-ffi)</summary>

###
[`v0.1.12`](https://togithub.com/getditto/safer_ffi/releases/tag/v0.1.12)

[Compare
Source](https://togithub.com/getditto/safer_ffi/compare/v0.1.11...v0.1.12)

#### What's Changed

- Fix `Bytes`' `ReprC` impl, as `vtable` is not necessarily aligned by
[@&#8203;p-avital](https://togithub.com/p-avital) in
[https://github.com/getditto/safer_ffi/pull/235](https://togithub.com/getditto/safer_ffi/pull/235)

**Full Changelog**:
getditto/safer_ffi@v0.1.11...v0.1.12

###
[`v0.1.11`](https://togithub.com/getditto/safer_ffi/releases/tag/v0.1.11)

[Compare
Source](https://togithub.com/getditto/safer_ffi/compare/v0.1.10...v0.1.11)

#### What's Changed

- Upgrade stabby to 36.1.1 by
[@&#8203;p-avital](https://togithub.com/p-avital) in
[https://github.com/getditto/safer_ffi/pull/234](https://togithub.com/getditto/safer_ffi/pull/234)

**Full Changelog**:
getditto/safer_ffi@v0.1.10...v0.1.11

###
[`v0.1.10`](https://togithub.com/getditto/safer_ffi/releases/tag/v0.1.10)

[Compare
Source](https://togithub.com/getditto/safer_ffi/compare/v0.1.9...v0.1.10)

#### What's Changed

- Add ability to emit type aliases to C headers by
[@&#8203;danielhenrymantilla](https://togithub.com/danielhenrymantilla)
in
[https://github.com/getditto/safer_ffi/pull/228](https://togithub.com/getditto/safer_ffi/pull/228)
- Fix FFI typedef pattern not supporting generics by
[@&#8203;danielhenrymantilla](https://togithub.com/danielhenrymantilla)
in
[https://github.com/getditto/safer_ffi/pull/229](https://togithub.com/getditto/safer_ffi/pull/229)
- 🙈 Exclude examples/point from workspace by
[@&#8203;phatblat](https://togithub.com/phatblat) in
[https://github.com/getditto/safer_ffi/pull/212](https://togithub.com/getditto/safer_ffi/pull/212)
- `raw_const` addition to `#[ffi_export]`-ed `const`s for SWIG compat by
[@&#8203;danielhenrymantilla](https://togithub.com/danielhenrymantilla)
in
[https://github.com/getditto/safer_ffi/pull/230](https://togithub.com/getditto/safer_ffi/pull/230)
- \[⚠️ technically breaking] Add interning to `Bytes`, reducing the need
for allocations when slices are small. by
[@&#8203;p-avital](https://togithub.com/p-avital) in
[https://github.com/getditto/safer_ffi/pull/231](https://togithub.com/getditto/safer_ffi/pull/231)
- Despite this being a technically breaking change, API-wise (`Bytes :
From<&'static [u8]>` rather than `From<&'any [u8]>`), and ABI-wise
(`Bytes`' `.vtable` pointer can now be an ill-alligned odd address
representing the bit-tagging of its now added inlined-bytes
representation), both of these cases are small enough, and `0.1.9` has
been out for enough of a short time, not to warrant a major bump.
`0.1.9` will be yanked in a couple weeks.
- Add convenience release scripts by
[@&#8203;danielhenrymantilla](https://togithub.com/danielhenrymantilla)
in
[https://github.com/getditto/safer_ffi/pull/232](https://togithub.com/getditto/safer_ffi/pull/232)

**Full Changelog**:
getditto/safer_ffi@v0.1.9...v0.1.10

</details>

<details>
<summary>serde-rs/json (serde_json)</summary>

###
[`v1.0.122`](https://togithub.com/serde-rs/json/releases/tag/v1.0.122)

[Compare
Source](https://togithub.com/serde-rs/json/compare/v1.0.121...v1.0.122)

- Support using `json!` in no-std crates
([#&#8203;1166](https://togithub.com/serde-rs/json/issues/1166))

</details>

---

### Configuration

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

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

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

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

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

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job log](https://developer.mend.io/github/dora-rs/dora).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
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