Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ jobs:
- name: Check formatting
run: cargo fmt --all --check

# TODO
# # Apply clippy lints
# clippy:
# name: clippy
# runs-on: ubuntu-latest
# steps:
# - uses: actions/checkout@v4
# - name: Apply clippy lints
# run: cargo clippy --all-features
clippy:
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ env.nightly }}
components: clippy
- uses: actions-rs-plus/clippy-check@v2
with:
args: --all --all-features --all-targets
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the set of clippy lints that have been enabled?

I think we want to pin this to a specific rustc to avoid random breakages on each new Rust version.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

with this config, the lints are clippy::all.

happy to pin rust to a specific version, the problem is remembering to bump this regularly. Renovate can do this, dependabot cant. not sure if the maintainers would be amenable to adding renovate just for this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ah i see there's already a pinned nightly and it's wildly out of date...

might look at addressing that separately

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

see #815

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not too worried about bumping the bytes crate clippy lint version. And clippy::all is a really really broad category.

Copy link
Copy Markdown
Author

@danieleades danieleades Jan 5, 2026

Choose a reason for hiding this comment

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

do you have a particular lint set in mind?

i think concerns about the broadness of the category are obviated by pinning the toolchain. any warnings you're not happy with can be selectively disabled, and new ones only creep in predictably on bumping the pinned version.

Comment thread
danieleades marked this conversation as resolved.
Outdated

# This represents the minimum Rust version supported by
# Bytes. Updating this should be done in a dedicated PR.
Expand Down
12 changes: 6 additions & 6 deletions benches/buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ struct TestBuf {
readlen: usize,
}
impl TestBuf {
fn new(buf: &'static [u8], readlens: &'static [usize], init_pos: usize) -> TestBuf {
let mut buf = TestBuf {
fn new(buf: &'static [u8], readlens: &'static [usize], init_pos: usize) -> Self {
let mut buf = Self {
buf,
readlens,
init_pos,
Expand Down Expand Up @@ -68,13 +68,13 @@ struct TestBufC {
inner: TestBuf,
}
impl TestBufC {
fn new(buf: &'static [u8], readlens: &'static [usize], init_pos: usize) -> TestBufC {
TestBufC {
fn new(buf: &'static [u8], readlens: &'static [usize], init_pos: usize) -> Self {
Self {
inner: TestBuf::new(buf, readlens, init_pos),
}
}
fn reset(&mut self) {
self.inner.reset()
self.inner.reset();
}
}
impl Buf for TestBufC {
Expand All @@ -84,7 +84,7 @@ impl Buf for TestBufC {
}
#[inline(never)]
fn advance(&mut self, cnt: usize) {
self.inner.advance(cnt)
self.inner.advance(cnt);
}
#[inline(never)]
fn chunk(&self) -> &[u8] {
Expand Down
20 changes: 10 additions & 10 deletions benches/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn deref_unique(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(&buf[..]);
}
})
});
}

#[bench]
Expand All @@ -26,7 +26,7 @@ fn deref_shared(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(&buf[..]);
}
})
});
}

#[bench]
Expand All @@ -37,7 +37,7 @@ fn deref_static(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(&buf[..]);
}
})
});
}

#[bench]
Expand All @@ -49,7 +49,7 @@ fn clone_static(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(test::black_box(&bytes).clone());
}
})
});
}

#[bench]
Expand All @@ -60,7 +60,7 @@ fn clone_shared(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(test::black_box(&bytes).clone());
}
})
});
}

#[bench]
Expand All @@ -72,7 +72,7 @@ fn clone_arc_vec(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(test::black_box(&bytes).clone());
}
})
});
}

#[bench]
Expand All @@ -82,7 +82,7 @@ fn from_long_slice(b: &mut Bencher) {
b.iter(|| {
let buf = Bytes::copy_from_slice(&data[..]);
test::black_box(buf);
})
});
}

#[bench]
Expand All @@ -93,7 +93,7 @@ fn slice_empty(b: &mut Bencher) {
for i in 0..1000 {
test::black_box(b.slice(i % 100..i % 100));
}
})
});
}

#[bench]
Expand All @@ -104,7 +104,7 @@ fn slice_short_from_arc(b: &mut Bencher) {
for i in 0..1000 {
test::black_box(b.slice(1..2 + i % 10));
}
})
});
}

#[bench]
Expand All @@ -116,5 +116,5 @@ fn split_off_and_drop(b: &mut Bencher) {
test::black_box(b.split_off(100));
test::black_box(b);
}
})
});
}
26 changes: 15 additions & 11 deletions benches/bytes_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ fn alloc_small(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(BytesMut::with_capacity(12));
}
})
});
}

#[bench]
fn alloc_mid(b: &mut Bencher) {
b.iter(|| {
test::black_box(BytesMut::with_capacity(128));
})
});
}

#[bench]
fn alloc_big(b: &mut Bencher) {
b.iter(|| {
test::black_box(BytesMut::with_capacity(4096));
})
});
}

#[bench]
Expand All @@ -38,7 +38,7 @@ fn deref_unique(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(&buf[..]);
}
})
});
}

#[bench]
Expand All @@ -57,7 +57,7 @@ fn deref_unique_unroll(b: &mut Bencher) {
test::black_box(&buf[..]);
test::black_box(&buf[..]);
}
})
});
}

#[bench]
Expand All @@ -70,7 +70,7 @@ fn deref_shared(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(&buf[..]);
}
})
});
}

#[bench]
Expand All @@ -86,7 +86,7 @@ fn deref_two(b: &mut Bencher) {
test::black_box(&buf1[..]);
test::black_box(&buf2[..]);
}
})
});
}

#[bench]
Expand All @@ -99,7 +99,7 @@ fn clone_frozen(b: &mut Bencher) {
for _ in 0..1024 {
test::black_box(&bytes.clone());
}
})
});
}

#[bench]
Expand All @@ -108,7 +108,7 @@ fn alloc_write_split_to_mid(b: &mut Bencher) {
let mut buf = BytesMut::with_capacity(128);
buf.put_slice(&[0u8; 64]);
test::black_box(buf.split_to(64));
})
});
}

#[bench]
Expand All @@ -125,7 +125,7 @@ fn drain_write_drain(b: &mut Bencher) {
}

test::black_box(parts);
})
});
}

#[bench]
Expand All @@ -141,7 +141,7 @@ fn fmt_write(b: &mut Bencher) {
unsafe {
buf.set_len(0);
}
})
});
}

#[bench]
Expand Down Expand Up @@ -255,6 +255,10 @@ fn put_u8_vec_push(b: &mut Bencher) {

b.bytes = cnt as u64;
b.iter(|| {
#[allow(
clippy::same_item_push,
reason = "Benchmarking the push operation itself, not Vec construction"
)]
for _ in 0..cnt {
buf.push(b'x');
}
Expand Down
Loading