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

Disable panic hooks in Release mode #889

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

Ughuuu
Copy link
Contributor

@Ughuuu Ughuuu commented Sep 9, 2024

Add a flag that does extra logging only when enabled.

Edit: This improves performance a lot. From my tests, doing a project https://github.com/Ughuuu/performance-test where I call a function 1 million times, I get the following times before and after in godot-rust:

  • Before this change: 70ms
  • After this change: 30ms

The function is a simple noop function.

@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-889

@andreymal
Copy link
Contributor

This partially addresses #845 (comment) so I like it

But there is no way to enable extra-logging with the godot crate (the godot-core crate should not be used directly, if I understand everything correctly)

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: core Core components labels Sep 9, 2024
@Bromeon Bromeon changed the title Add extra logging Disable panic hooks in Release mode Sep 9, 2024
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thank you!

Since the PR is not in draft: please squash the commits 🙂

godot-core/src/private.rs Outdated Show resolved Hide resolved
godot-core/src/private.rs Outdated Show resolved Hide resolved
godot-core/src/private.rs Outdated Show resolved Hide resolved
godot-core/src/private.rs Outdated Show resolved Hide resolved
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Sep 9, 2024

Done.

@Ughuuu Ughuuu marked this pull request as draft September 9, 2024 15:49
@Bromeon
Copy link
Member

Bromeon commented Sep 9, 2024

Please add periods/fullstops at the end of comments. You can also change existing ones.

Update private.rs

Update private.rs

Update private.rs

Update private.rs

Update Cargo.toml

Update private.rs

Update private.rs

Update private.rs
@Ughuuu Ughuuu marked this pull request as ready for review September 9, 2024 18:05
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Sep 9, 2024

Ok, look good from my side to be merged.

@Bromeon Bromeon added this pull request to the merge queue Sep 9, 2024
Merged via the queue into godot-rust:master with commit 6e6889a Sep 9, 2024
15 checks passed
@Ughuuu Ughuuu deleted the add-extended-logging branch September 9, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Core components quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants