Added Custom Performance Monitor

[simpuid]

Task 1 & 2 of GSoC project

Custom monitors can be added/removed/check using Performance.add_custom_monitor/Performance.remove_custom_monitor/Performance.has_custom_monitor

The value can be viewed in the Custom section of Monitor tab of Debugger.
Basically...

func _enter_tree():
	var self_var = self
	Performance.add_custom_monitor("Hello/There", Callable(self_var, "monitor_call"), [1, 2])
	Performance.add_custom_monitor("", Callable(self_var, "monitor_call"), [1, 2])
	Performance.add_custom_monitor("General/Kenobi", Callable(self_var, "monitor_call"), [2, 3])
	Performance.add_custom_monitor("Multiple/First", Callable(self_var, "monitor_call"), [3, 4])
	Performance.add_custom_monitor("Multiple/Second", Callable(self_var, "monitor_call"), [4, 5])
	Performance.add_custom_monitor("Audio/BuiltinCategory", Callable(self_var, "monitor_call"), [5, 6])
	Performance.add_custom_monitor("Too/Much/Slashes/So it's in Custom", Callable(self_var, "monitor_call"), [6, 7])
	Performance.add_custom_monitor("No slash so it's in Custom", Callable(self_var, "monitor_call"), [7, 8])

func _exit_tree():
	Performance.remove_custom_monitor("Hello/There")
	Performance.remove_custom_monitor("")
	Performance.remove_custom_monitor("General/Kenobi")
	Performance.remove_custom_monitor("Multiple/First")
	Performance.remove_custom_monitor("Multiple/Second")
	Performance.remove_custom_monitor("Audio/BuiltinCategory")
	Performance.remove_custom_monitor("Too/Much/Slashes/So it's in Custom")
	Performance.remove_custom_monitor("No slash so it's in Custom")

func monitor_call(from, to) -> float:
	return rand_range(from, to)

Above code causes

Edit: Monitor list is now cleared when debugging is started.
Edit: Updated with categorized monitors.
Edit: This implements godotengine/godot-proposals#1014 too

Bugsquad edit: This closes godotengine/godot-proposals#229.

[samdze]

Nice!
Would it be possible to add the capability to define monitors inside any category in the monitors tree? (Not only in the arbitrary "Custom" one)

Like this:

func _enter_tree():
	var self_var = self

        // This one goes under "Ammo"
	Performance.add_custom_monitor("Ammo/Gun", ..., ...)
        Performance.add_custom_monitor("Ammo/Rifle", ..., ...)

        // These one go under another category "Enemies"
	Performance.add_custom_monitor("Enemies/Bats", ..., ...)
        Performance.add_custom_monitor("Enemies/Spiders", ..., ...)

        // And you could also add a monitor to an already existing category
	Performance.add_custom_monitor("Physics 3d/Bouncing", ..., ...)

        // The method call could be changed this way if preferred:
        Performance.add_custom_monitor(category, name, callable, params)

I think the user would be able to organize his monitors much better.

Maybe I'd also drop the "_custom" from the method names, there's no need to specify those are "custom" monitors that you're adding/removing.

[simpuid]

add_monitor("category/name" , callable...., params....) seems the best option.

add_monitor("category", "name", callable..., params....) will cause problems because internally the ids used for those monitors would be something like category/name(That slash could be any ASCII symbol). Method calls like Performance.add_monitor("a/b", "c", callable..., params...) would cause confusion among users.

I think something like this will work

• / is present => use the supplied category
• / is absent => use the Custom category.

Adding monitors to built-in categories seems difficult but I think I can do it. However keep in mind that there is no Physics 3d . The Performance class uses snake_case internally i.e. physics_3d is converted to Physics 3d, so something like physics_3d/bouncing will use the builtin category. This opens the issue of name collisions with monitors like Physics 3d/bouncing with physics_3d/bouncing
@Faless @mhilbrunner What are your opinions about categorizing custom monitors?

[simpuid]

Accidently clicked the Close and comment button

[simpuid]

@samdze Went ahead with Capitalized category names. (Physics 3d/something will work now).
The only limitation is that Custom/something and something both will be displayed in Custom category.

[samdze]

@simpuid Seems good!

I also saw that renaming the methods to add_monitor(...), get_monitor(...) and has_monitor(...) would collide with the already existing get_monitor(Monitor p_monitor), used for built-in monitors.
So while I'd prefer not having that "_custom" in the method names, it's probably necessary if a more extensive refactor is unwanted.

[samdze]

I also opened this proposal, you may be interested.
godotengine/godot-proposals#1014

[Faless]

This looks great! Excellent work 🏅 . I'm wondering if we should move the performance drawing functions (setup and parsing) to its own EditorDebuggerPerformance class. What do you think @simpuid ?

EDIT: That would also help keeping things clean if we implement godotengine/godot-proposals#1014 in the future (which I think it's a good idea).

[simpuid]

This looks great! Excellent work medal_sports . I'm wondering if we should move the performance drawing functions (setup and parsing) to its own EditorDebuggerPerformance class. What do you think @simpuid ?

EDIT: That would also help keeping things clean if we implement godotengine/godot-proposals#1014 in the future (which I think it's a good idea).

Yes, we can do that (just like EditorProfiler, EditorVisualProfiler and EditorNetworkProfiler). I can also take help from their implementations.

Shouldn't we name it EditorPerformance / EditorPerformanceProfiler? (there is no "debugger" word in EditorProfiler, EditorVisualProfiler and EditorNetworkProfiler)

[Faless]

Shouldn't we name it EditorPerformance / EditorPerformanceProfiler?

Yeah, I agree, both sounds good.
Maybe the latter is more in line with RemoteDebugger::PerformanceProfiler in remote_debugger.cpp

[simpuid]

Yeah, I agree, both sounds good.
Maybe the latter is more in line with RemoteDebugger::PerformanceProfiler in remote_debugger.cpp

@Faless I rewrote the parsing and drawing logic to a separate class EditorPerformanceProfiler
The implementation deviates from the proposal but it's better.
Changes:

• Built-in performance monitors' data are stored the same way as custom monitor.
• There is only one drawing function EditorPerformanceProfiler::_monitor_draw, Improve readability and data exploring capabilities of monitors and profiler godot-proposals#1014 could be implemented there for both type of monitors.
• There are only 2 types of messages instead of three
  1. 'performance:profile_frame' : contains the data of all (builtin + custom) monitors.
  2. 'performance:profile_names' : contains the name of custom monitors.

[simpuid]

@samdze Added horizontal guide lines and a vertical marker line that can be placed by clicking LMB.
I think that should be enough for godotengine/godot-proposals#1014

Does text of horizontal guide lines look too cumbersome?

[Calinou]

@simpuid There seems to be a lot of horizontal lines on your graph. My PR made it so the number of lines depends on the monitor's height (and never exceeds 5).

[simpuid]

@simpuid There seems to be a lot of horizontal lines on your graph. My PR made it so the number of lines depends on the monitor's height (and never exceeds 5).

I am calculating the number of horizontal lines depending on the graph height and text height (So they can be rendered easily)

int line_count = rect.size.height / (graph_font->get_height() * 2);

@Calinou should I limit it to max 5?

[Calinou]

@Calinou should I limit it to max 5?

Yes, there's not much benefit to having more than 5.

[simpuid]

Added documentation

[Faless]

Looking good! Amazing job 🏆 !

[Faless]

Actually, I think you accidentally committed ProjectSetting.xml, could you revert that? Thanks

[simpuid]

Actually, I think you accidentally committed ProjectSetting.xml, could you revert that? Thanks

Sorry! Reverted.

[akien-mga]

Thanks!

[neikeq]

Your custom monitors will make a fine addition to our collection.

[williamd1k0]

Sorry, but is this portable for 3.2 branch? :P

[simpuid]

Sorry, but is this portable for 3.2 branch? :P

I don't think so! Callables are not available in 3.2 branch :(

[Faless]

Sorry, but is this portable for 3.2 branch?

Not really, the whole debugger was heavily refactored in the master branch, backporting is unlikely.