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

refactor: inline handle_panic into macro output #2158

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

davidhewitt
Copy link
Member

@davidhewitt davidhewitt commented Feb 10, 2022

I was looking at cargo llvm-lines output one last time, and a major contributor to llvm line count seems to be pyo3::callback::handle_panic.

Because it takes the whole function body as a closure, it's going to be instantiated separately for each pyfunction. Inlining it into the macro outputs leads to about a 10% reduction in llvm line counts for the pytests crate.

cd pytests; cargo llvm-lines | head -n 10 on main:

  Lines         Copies       Function name
  -----         ------       -------------
  59320 (100%)  2227 (100%)  (TOTAL)
   3546 (6.0%)    62 (2.8%)  std::panicking::try
   3223 (5.4%)    62 (2.8%)  pyo3::callback::handle_panic
   2341 (3.9%)    56 (2.5%)  <core::result::Result<T,E> as core::ops::try_trait::Try>::branch
   2046 (3.4%)    62 (2.8%)  std::panicking::try::do_catch
   1820 (3.1%)    10 (0.4%)  pyo3::types::module::PyModule::add_wrapped
   1759 (3.0%)    62 (2.8%)  std::panicking::try::do_call
   1647 (2.8%)     3 (0.1%)  pyo3::impl_::extract_argument::FunctionDescription::extract_arguments_tuple_dict

On this branch:

  Lines         Copies       Function name
  -----         ------       -------------
  54716 (100%)  1987 (100%)  (TOTAL)
   2963 (5.4%)    51 (2.6%)  std::panicking::try
   2341 (4.3%)    56 (2.8%)  <core::result::Result<T,E> as core::ops::try_trait::Try>::branch
   1820 (3.3%)    10 (0.5%)  pyo3::types::module::PyModule::add_wrapped
   1683 (3.1%)    51 (2.6%)  std::panicking::try::do_catch
   1647 (3.0%)     3 (0.2%)  pyo3::impl_::extract_argument::FunctionDescription::extract_arguments_tuple_dict
   1473 (2.7%)    51 (2.6%)  std::panicking::try::do_call
   1342 (2.5%)    17 (0.9%)  pyo3::impl_::extract_argument::extract_argument

@davidhewitt davidhewitt merged commit 08d983b into PyO3:main Feb 10, 2022
@davidhewitt davidhewitt deleted the inline-handle-panic branch February 10, 2022 21:28
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.

1 participant