Skip to content

Conversation

@davidhewitt
Copy link
Member

@davidhewitt davidhewitt commented Aug 2, 2025

Inspired by #5223 (comment), this cleans up the Python docstring generation by moving the work to compile time. Avoids a runtime allocation in startup as well as removes a use of a GILOnceCell static cache, for each #[pyclass].

The interesting new machinery is in src/impl_/pyclass/doc.rs. It is responsible for assembling the pieces of the docstring together according to what's available, and then the proc macros invoke the const concat machinery (thanks @Tpt) to build the docstring into the binary.

@davidhewitt davidhewitt marked this pull request as ready for review August 8, 2025 20:04
Comment on lines -9 to +14
const L: &'static str = $l;
const R: &'static str = $crate::impl_::concat::const_concat!($($r),*);
const LEN: usize = L.len() + R.len();
const fn combine(l: &'static [u8], r: &'static [u8]) -> [u8; LEN] {
let mut out = [0u8; LEN];
let mut i = 0;
while i < l.len() {
out[i] = l[i];
i += 1;
const PIECES: &[&str] = &[$l, $($r),+];
const RAW_BYTES: &[u8] = &$crate::impl_::concat::combine_to_array::<{
$crate::impl_::concat::combined_len(PIECES)
}>(PIECES);
// Safety: `RAW_BYTES` is combined from valid &str pieces
unsafe { ::std::str::from_utf8_unchecked(RAW_BYTES) }
Copy link
Member Author

Choose a reason for hiding this comment

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

I rewrote this a bit so that instead of needing to apply the machinery recursively to concatenate pairs, it concatenates a whole slice of pieces.

I also moved as much machinery as possible out into the const fn helpers below. There are some for str inputs and some for bytes inputs (maybe one day with const AsRef<[u8]> implementations we could avoid the duplication).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you! I think we can migrate the introspection code to use directly these const functions and drop the macro entirely. It might allow us to only use the byte variant of the functions and drop the string variants entirely. Happy to do it in a follow-up.

@davidhewitt davidhewitt mentioned this pull request Aug 8, 2025
Copy link
Contributor

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

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

Quite some const layers to peel off 😅, but pretty clever solution. LGTM

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 9, 2025

CodSpeed Performance Report

Merging #5286 will degrade performances by 13.78%

Comparing davidhewitt:compile-time-class-doc (9ff5bd3) with main (d265254)

Summary

⚡ 1 improvements
❌ 3 regressions
✅ 97 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
extract_int_cast_fail 245.6 ns 274.7 ns -10.62%
extract_str_cast_fail 182.5 ns 211.7 ns -13.78%
extract_str_cast_success 584.2 ns 525.8 ns +11.09%
not_a_list_via_cast 182.5 ns 211.7 ns -13.78%

@davidhewitt davidhewitt enabled auto-merge August 9, 2025 18:40
@davidhewitt davidhewitt added this pull request to the merge queue Aug 9, 2025
Merged via the queue into PyO3:main with commit c366646 Aug 9, 2025
41 of 43 checks passed
@davidhewitt davidhewitt deleted the compile-time-class-doc branch August 9, 2025 20:14
Tpt added a commit to Tpt/pyo3 that referenced this pull request Aug 13, 2025
github-merge-queue bot pushed a commit that referenced this pull request Aug 13, 2025
* Drop const_concat! macro and str-focused const combine functions

Follow up to #5286

* Makes combine not `pub`
Rafa-Gu98 pushed a commit to Rafa-Gu98/pyo3 that referenced this pull request Aug 14, 2025
* move `#[pyclass]` docstring generation to compile time

* improve error message

* clean up to use cstr all the way through

* fixup msrv

* fixup final tests, newsfragment

* runtime test coverage

* use "copy_from_slice" (with a note for the future)

* fix clippy beta
Rafa-Gu98 pushed a commit to Rafa-Gu98/pyo3 that referenced this pull request Aug 14, 2025
…3#5315)

* Drop const_concat! macro and str-focused const combine functions

Follow up to PyO3#5286

* Makes combine not `pub`
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.

3 participants