Skip to content

refactor to remove add_to_module functions from generated code#4009

Merged
davidhewitt merged 3 commits intoPyO3:mainfrom
davidhewitt:add-to-module
Mar 29, 2024
Merged

refactor to remove add_to_module functions from generated code#4009
davidhewitt merged 3 commits intoPyO3:mainfrom
davidhewitt:add-to-module

Conversation

@davidhewitt
Copy link
Copy Markdown
Member

@davidhewitt davidhewitt commented Mar 29, 2024

Related to #3989

@alex / @reaperhulk, want to try this branch with the cryptography CI and see if it fixes (or reduces) the problem?

This reshapes the generated macro output so that it only emits constants (no functions) and then we leave the implementations inside PyO3's impl_ module, as I suggested on that issue. This is probably better for binary size / compile times even if it doesn't help the coverage problem.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Mar 29, 2024

CodSpeed Performance Report

Merging #4009 will not alter performance

Comparing davidhewitt:add-to-module (9de7016) with main (dd17102)

🎉 Hooray! codspeed-rust just leveled up to 2.4.0!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 68 untouched benchmarks

@alex
Copy link
Copy Markdown
Member

alex commented Mar 29, 2024

This significantly improved the coverage situation. We still have a few things missing coverage:

  • All invocations of pyo3::import_exception
  • Exactly one pyclass
  • One other bizzaro line (that's probably not related to pyo3)

@alex
Copy link
Copy Markdown
Member

alex commented Mar 29, 2024

For clarity, I think it makes sense to merge this and then we can do follow up PRs resolving the other coverage issues.

@davidhewitt
Copy link
Copy Markdown
Member Author

Great, will merge now!

@davidhewitt davidhewitt enabled auto-merge March 29, 2024 11:53
@davidhewitt davidhewitt added this pull request to the merge queue Mar 29, 2024
Merged via the queue into PyO3:main with commit cf74624 Mar 29, 2024
@davidhewitt davidhewitt deleted the add-to-module branch March 29, 2024 13:30
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