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

Display functions that were previously forgotten in Profiler #86772

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Jan 4, 2024

The array data in add() contains these data, but some data may have been forgotten to be converted into ServerFunctionInfo.

This results in some information not being displayed in the Profiler.

Before This PR
0 1

Should the display of these functions be reordered? According to the order in the same frame. 🤔

@akien-mga akien-mga added bug topic:editor cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jan 4, 2024
@akien-mga akien-mga requested a review from a team January 4, 2024 09:11
@akien-mga akien-mga added this to the 4.3 milestone Jan 4, 2024
The array data in `add()` contains these data, but some data may
have been forgotten to be converted into `ServerFunctionInfo`.

This results in some information not being displayed in the Profiler.
@Rindbee Rindbee force-pushed the display-functions-in-profiler branch from ef8d6a8 to 3d3c4e8 Compare January 4, 2024 09:20
@akien-mga
Copy link
Member

Should the display of these functions be reordered? According to the order in the same frame. 🤔

What would be the frame order?
Or conversely, where does the current order stem from?

@Rindbee
Copy link
Contributor Author

Rindbee commented Jan 4, 2024

PhysicsServer2D::get_singleton()->flush_queries();

PhysicsServer2D::get_singleton()->step(physics_step * time_scale);

The array data is sorted and passed when calling GodotPhysicsServer2D::flush_queries(), but flush_queries() is not at the end of the physical frame, so the data in the array is not the same frame's data.

for (int i = 0; i < GodotSpace2D::ELAPSED_TIME_MAX; i++) {
values[i * 2 + 0] = time_name[i];
values[i * 2 + 1] = USEC_TO_SEC(total_time[i]);
}
values.push_back("flush_queries");
values.push_back(USEC_TO_SEC(OS::get_singleton()->get_ticks_usec() - time_beg));
values.push_front("physics_2d");
EngineDebugger::profiler_add_frame_data("servers", values);

So in a sense, the current data is also sorted according to chronological order.

@philipcass
Copy link

philipcass commented Jan 19, 2024

What's currently preventing this PR being merged? Would like to see this added as this also fixes usage of EngineDebugger.profiler_add_frame_data (Currently, only a single call will ever display using this method)

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

LGTM, great work 🏆

@YuriSizov YuriSizov merged commit c721067 into godotengine:master Jan 22, 2024
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@Rindbee Rindbee deleted the display-functions-in-profiler branch January 22, 2024 23:06
@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants