Skip to content

fix exception handling on Python 3.12#3306

Merged
adamreichold merged 1 commit intoPyO3:mainfrom
davidhewitt:3.12-exceptions
Jul 20, 2023
Merged

fix exception handling on Python 3.12#3306
adamreichold merged 1 commit intoPyO3:mainfrom
davidhewitt:3.12-exceptions

Conversation

@davidhewitt
Copy link
Copy Markdown
Member

@davidhewitt davidhewitt commented Jul 10, 2023 •
edited
Loading

Reworked exception handling for Python 3.12. Ref #3305

The simplifications to PyErrState result in a ~10-15% perf improvement in the bench_err benchmarks on all Python versions. On 3.12 the enum size has been reduced from 32 to 24 bytes, which might have small global perf benefits too.

While working on this I noticed a bug on <3.12 where the original panic message is not retrieved properly in PyErr::fetch if the PanicException is normalised. I've added that as a second commit.

alex
alex reviewed Jul 10, 2023
View reviewed changes
Comment thread src/err/mod.rs Outdated
@davidhewitt davidhewitt mentioned this pull request Jul 11, 2023
Closed
@davidhewitt davidhewitt marked this pull request as ready for review July 11, 2023 07:48
@davidhewitt
Copy link
Copy Markdown
Member Author

davidhewitt commented Jul 11, 2023

Pushed a more complete implementation which I'm happy with, and updated OP.

davidhewitt
davidhewitt commented Jul 11, 2023
View reviewed changes
Comment thread src/err/err_state.rs
Comment thread src/err/mod.rs
Comment thread src/err/mod.rs Outdated
Comment thread src/err/mod.rs Outdated
davidhewitt
davidhewitt commented Jul 11, 2023
View reviewed changes
Comment thread src/exceptions.rs
@davidhewitt
Copy link
Copy Markdown
Member Author

davidhewitt commented Jul 11, 2023

I think best if I circle back to this later to split out into multiple PRs for the changes of behaviour / fixes etc.

mejrs
mejrs reviewed Jul 11, 2023
View reviewed changes
Comment thread src/err/err_state.rs Outdated
This was referenced Jul 11, 2023
Merged
Merged
@davidhewitt davidhewitt force-pushed the 3.12-exceptions branch 2 times, most recently from d9177ab to 7c65b98 Compare July 14, 2023 20:38
@davidhewitt
Copy link
Copy Markdown
Member Author

davidhewitt commented Jul 14, 2023

Assuming CI is green, I believe this is now good to review.

@davidhewitt
Copy link
Copy Markdown
Member Author

davidhewitt commented Jul 16, 2023

On further reflection, I should be able to split this out to first make the PyErrState::LazyValue & PyErrState::LazyTypeAndValue -> PyErrState::Lazy change a separate PR, if that would help with reviewing.

@adamreichold
Copy link
Copy Markdown
Member

adamreichold commented Jul 16, 2023

if that would help with reviewing.

I am pretty swamped at the moment, so anything that turns this into smaller chunks would be appreciated.

@davidhewitt davidhewitt mentioned this pull request Jul 16, 2023
Merged
@davidhewitt
Copy link
Copy Markdown
Member Author

davidhewitt commented Jul 16, 2023

if that would help with reviewing.

I am pretty swamped at the moment, so anything that turns this into smaller chunks would be appreciated.

👍 #3323

@davidhewitt davidhewitt force-pushed the 3.12-exceptions branch 2 times, most recently from 173dc9d to 291b2e5 Compare July 18, 2023 07:42
@davidhewitt davidhewitt mentioned this pull request Jul 18, 2023
Merged
@davidhewitt
Copy link
Copy Markdown
Member Author

davidhewitt commented Jul 18, 2023

Ok, I think this is as small a diff as it can possibly get. I split off the second commit into #3326.

@davidhewitt davidhewitt mentioned this pull request Jul 19, 2023
Closed
adamreichold
adamreichold reviewed Jul 19, 2023
View reviewed changes
Comment thread src/err/mod.rs Outdated
adamreichold
adamreichold reviewed Jul 19, 2023
View reviewed changes
Comment thread src/err/mod.rs
adamreichold
adamreichold reviewed Jul 19, 2023
View reviewed changes
Comment thread src/err/mod.rs Outdated
adamreichold
adamreichold reviewed Jul 19, 2023
View reviewed changes
Comment thread src/err/mod.rs Outdated
@adamreichold adamreichold added this pull request to the merge queue Jul 20, 2023
Merged via the queue into PyO3:main with commit 9e87aac Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alex alex alex left review comments

@mejrs mejrs mejrs left review comments

@adamreichold adamreichold adamreichold approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@davidhewitt @adamreichold @alex @mejrs