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.

Sign up for GitHub

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

Jump to bottom

Frequently asked questions #5

Closed
2 tasks
ghost opened this issue Jul 3, 2020 · 3 comments · Fixed by #33
Closed
2 tasks

Frequently asked questions #5

ghost opened this issue Jul 3, 2020 · 3 comments · Fixed by #33

Comments

@ghost
Copy link

ghost commented Jul 3, 2020

  • Instance
  • User-data errors (BorrowFailed, DifferentThread)

...and more?
@ghost ghost mentioned this issue Jul 3, 2020
Open
13 tasks
@bluenote10
Copy link
Contributor

Perhaps another topic for the FAQ: How to use the Godot profiler for native functions?

From what I can see (and also this Q/A), native functions/classes are not automatically visible in the profiler. Since the majority of my business logic runs in Rust, the output of Godot's built-in profiler isn't very useful so far. This example suggests, that on C++ side there is a Godot::gdnative_profiling_add_data(signature, t) function, which looks like providing functionality for manually adding profiling data (which wouldn't be too bad). What I couldn't figure out so far is where it is coming from, and if there is an equivalent in the Rust bindings.

@ghost
Copy link
Author

ghost commented Aug 24, 2020 •
edited by ghost
Loading

I don't think there is a high-level interface to gdnative_profiling_add_data in the Rust bindings yet. We might be able to add one, but native profilers (e.g. perf) will probably be much more accurate and informative than Godot's internal profiling when it comes to profiling Rust code. If we decide to add the profiling topic to the FAQ, I would prefer that native profilers be recommended.

@bluenote10
Copy link
Contributor

native profilers (e.g. perf) will probably be much more accurate and informative than Godot's internal profiling when it comes to profiling Rust code.

Good point. I guess there are use cases for both. What is nice about the built-in profiler:

  • easy to compare GDScript vs GDNative runtime.
  • understand the notion of a frame, which can be useful.
  • easy to use, just a mouse click away and convenient output.

I'll try to contribute a minimal integration...

@bluenote10 bluenote10 mentioned this issue Aug 24, 2020
Merged
bors bot referenced this issue in godot-rust/gdnative Aug 31, 2020
@bors @bluenote10
Merge #573
9777961 
573: Minimal integration of profiling api r=toasteater a=bluenote10

Following up on godot-rust/book#5, this is a naive way to integrate the profiling API, but it seems to work:

```rust
// Usage:
godot::nativescript::utils::profiling_add_data("::100::foo", 1000);
```

![image](https://user-images.githubusercontent.com/3620703/91095098-1318e600-e65c-11ea-9519-b5f30d948802.png)

Note: I think the API JSON actually has a small bug:

```json
          {
            "name": "godot_nativescript_profiling_add_data",
            "return_type": "void",
            "arguments": [
              ["const char *", "p_signature"],
              ["uint64_t", "p_line"]
            ]
          }
```
Note that the second argument says `p_line`, but it really is the time in microseconds. I think the name confusion stems from its usage example [here](https://github.com/godotengine/godot-cpp/blob/master/include/core/GodotProfiling.hpp).

Co-authored-by: Fabian Keller <github.100.fkeller@spamgourmet.com>
@jacobsky jacobsky mentioned this issue Sep 7, 2021
Merged
jacobsky added a commit to jacobsky/book that referenced this issue Sep 7, 2021
@jacobsky
Updated the FAQ and changed some of the formatting.
af92df6 
Added an additional Q/A that I missed that would probably be useful.

Added profiling FAQ to close godot-rust#5

Also added a fix to have the logging recipe get added to the summary as it was previously missing.
jacobsky added a commit to jacobsky/book that referenced this issue Sep 8, 2021
@jacobsky
Updated the FAQ and changed some of the formatting.
19ac1a9 
Added an additional Q/A that I missed that would probably be useful.

Added profiling FAQ to close godot-rust#5

Also added a fix to have the logging recipe get added to the summary as it was previously missing.
@Bromeon Bromeon linked a pull request Sep 8, 2021 that will close this issue
Updated the FAQ and changed some of the formatting. #33
Merged
jacobsky added a commit to jacobsky/book that referenced this issue Sep 8, 2021
@jacobsky
Updated the FAQ and changed some of the formatting.
aa77f87 
Added an additional Q/A that I missed that would probably be useful.

Added profiling FAQ to close godot-rust#5

Also added a fix to have the logging recipe get added to the summary as it was previously missing.
KarimHamidou referenced this issue in KarimHamidou/godot-rust Feb 6, 2023
@KarimHamidou @bluenote10
Merge #573
5452067 
573: Minimal integration of profiling api r=toasteater a=bluenote10

Following up on godot-rust/book#5, this is a naive way to integrate the profiling API, but it seems to work:

```rust
// Usage:
godot::nativescript::utils::profiling_add_data("::100::foo", 1000);
```

![image](https://user-images.githubusercontent.com/3620703/91095098-1318e600-e65c-11ea-9519-b5f30d948802.png)

Note: I think the API JSON actually has a small bug:

```json
          {
            "name": "godot_nativescript_profiling_add_data",
            "return_type": "void",
            "arguments": [
              ["const char *", "p_signature"],
              ["uint64_t", "p_line"]
            ]
          }
```
Note that the second argument says `p_line`, but it really is the time in microseconds. I think the name confusion stems from its usage example [here](https://github.com/godotengine/godot-cpp/blob/master/include/core/GodotProfiling.hpp).

Co-authored-by: Fabian Keller <github.100.fkeller@spamgourmet.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Updated the FAQ and changed some of the formatting. jacobsky/book
1 participant
@bluenote10