Opened 18 months ago

Closed 17 months ago

Last modified 17 months ago

#4056 closed enhancement (fixed)

[PATCH] Improve Profiler2

Reported by: wraitii Owned by:
Priority: Nice to Have Milestone: Alpha 21
Component: Core engine Keywords: patch
Cc: Patch:

Description

We currently have 2 in-game profilers. Profiler1 can be seen from in-game and gives you average results, which is rather useful to know if something is generally too slow. Profiler2 returns events as an absolute amount of time, and the http server is very useful if you want to see spikes, but rather rubbish otherwise. The problem is that right now we profile too many things and the buffer gets overwritten very quickly, which leads to a rather limited usefulness. The presentation of things is also not the best.

It would be good to improve Profiler2 to have more functionality and provide better analysis, such as full game profiling.

Attachments (4)

Profiler2.patch (22.8 KB) - added by wraitii 18 months ago.
Profiler2 core changes.
Profiler-scripts.patch (40.2 KB) - added by wraitii 18 months ago.
Apply patch to profile script components
profiler-fixheader.patch (17.0 KB) - added by wraitii 18 months ago.
Fix mistaken header in script.interface: should be SVN compatible
Profiler2.2.patch (47.7 KB) - added by wraitii 17 months ago.
Updated version

Download all attachments as: .zip

Change History (13)

Changed 18 months ago by wraitii

Attachment: Profiler2.patch added

Profiler2 core changes.

Changed 18 months ago by wraitii

Attachment: Profiler-scripts.patch added

Apply patch to profile script components

Changed 18 months ago by wraitii

Attachment: profiler-fixheader.patch added

Fix mistaken header in script.interface: should be SVN compatible

comment:1 Changed 18 months ago by wraitii

Keywords: patch review added

Please review only the fixheader patch. It should compile. If it does, it should be committed as the header is useless in ScriptInterface?.h and makes compile time on changes in the profiler really big (it still is after the patch, mind you).

The other 2 patches improve the functionality of Profiler2. Changes, Cpp side: -duration of profiling is stored relative to the last sync marker instead of the last event. This seems to fix weirdness in the output and should probably be done anyhow -Add a new struct type that only writes in the main buffer if the time taken is above a certain threshold. In the meantime, it writes to a temp buffer. There are 8 levels of buffer, so you can nest 8 of them (warning: this can happen easily if things become recursive). Can crash though, needs a bit more work.

The usefulness of this last struct is that you may want to profile "spikes" in some functions but not clutter the buffer (as it's only 1MB right now). So with that function, you can.

Generally speaking I think the profiler threads should have more state, such as a region that compiles events that are named similarly, perhaps? It can be useful. Also add a few mostly useless things

JS side: -fixed a bug (precision was too low, errored out and showed nothing) -the frame canvas on top is rendered on a logarithmic scale. -Added a canvas but it's mostly broken tbh. -Added a button to save current buffer JSON to a file, which you can then read offline. -Changed the zoom handlers so the zoom is more usable -WIP function to output some stats but maybe Profiler1 should do that.

My JS changes are mostly useless (except the scale things and the bug)

Changed 17 months ago by wraitii

Attachment: Profiler2.2.patch added

Updated version

comment:2 Changed 17 months ago by wraitii

Update version allows showing 2 useful graphs and compare replays. Still WIP.

comment:3 Changed 17 months ago by wraitii

In 18424:

Fix a double to float coercion and JS errors with empty threads in Profiler2 . Refs #4056

comment:4 Changed 17 months ago by wraitii

In 18435:

Fix crash after opening profiler2 with GPU profiling following r18423. Refs #4056

comment:5 Changed 17 months ago by wraitii

In 18436:

Improve Profiler2 JS interface with multiple threads and frameless threads. Add a setting to the analysis graphs. Refs #4056

comment:6 Changed 17 months ago by elexis

Keywords: performance removed
Summary: Improve Profiler2[PATCH] Improve Profiler2

Got a compile warn http://pastebin.com/q3mDmJdD, is it that http://pastebin.com/RzTzentF ? review keyword can go I suppose?

comment:7 Changed 17 months ago by wraitii

In 18442:

Fix compiler warning, thanks to elexis. Refs #4056

comment:8 Changed 17 months ago by wraitii

Keywords: review removed
Resolution: fixed
Status: newclosed

comment:9 Changed 17 months ago by elexis

(The actual feature was introduced in r18423.)

Note: See TracTickets for help on using tickets.