Add the ability to use Tracy plots directly from Lua via UnsyncedCtrl Spring.LuaTracyPlotConfig and Spring.LuaTracyPlot

[Beherith]

/*** Initialize a plot in Tracy for use in debugging or profiling
 *
 * @function Spring.LuaTracyPlotConfig
 * @string plotName which should be initialized
 * @string[opt] plotFormatType "Number"|"Percentage"|"Memory", default "Number"
 * @bool[opt] step stepwise chart, default true is stepwise
 * @bool[opt] fill color fill, default false is no fill
 * @number[opt] color unit32 number as BGR color, default white
 * @treturn nil
 */

/*** Update a Tracy Plot with a value
 *
 * @function Spring.LuaTracyPlot
 * @string plotName which LuaPlot should be updated (must have been initialized via LuaTracyPlotConfig)
 * @number plotvalue the number to show on the Tracy plot
 * @treturn nil
 */

I was now quickly able to plot chobby memory usage in a very fine grained way by wrapping a function via debug.sethook(), example below:

local function plotMem()
	if tracy then
		local ramuse = gcinfo()
		Spring.LuaTracyPlot("ChobbyMem",  ramuse)
	end
end
local function GetUserControls(userName,opts)
	debug.sethook(plotMem, 'c r')
	GetUserControlsWrapped(userName, opts)
	debug.sethook()
end

Result:

Million thanks to @sprunk !

[sprunk]

How tested was this? It doesn't compile.

Edit: fixed 54f90ce

[sprunk]

Also, the set was originally there to prevent TracyPlot from being called on names that weren't initialized via TracyPlotConfig but #1215 (comment) seems to say that isn't actually a problem (it will just use default config). So the set can be dropped altogether. I added a commit as well. Feel free to drop any of the last 3 if there's an issue with them.

[p2004a]

@sprunk I believe your change is incorrect. Whatever the plot is configured or not and whatever it's fine to use dynamically allocated strings that can be freed are orthogonal.

Tracy in almost all cases is using the pointed to the string literal as identifier. It's passing only the pointer to the worker thread, and then the actual string value is fetched afterwards at some later time. I believe in case of zones, the unique pointer is required, in case of plots the docs say

It is beneficial but not required to use a unique pointer for name string literal (see section 3.1.2 for more details).

Which is the same section that explains the string literals for Zones. Even if the unique pointer for strings was not required, I'm quite sure it must not be memory that can be freed during program runtime.

[sprunk]

Thanks, I did a switcheroo and replaced that commit with one that puts your comment in the code.

[p2004a]

I've realized one more thing, if we want to maintain (And I think we want) that tracing annotations are 0 overhead in release build, those exports should be in #if defined(TRACY_ENABLE) block

The problem is that tracy::LuaRemove calls we have to drop tracing in the release mode, won't delete Spring.LuaTracyPlot calls, so we might need to add our own cleanup like the code in tracy to do that...

[sprunk]

Relatedly, maybe these should go under the tracy namespace in Lua?

I've seen that the tracy cleanup is actually just a dumb removal of tracy.Blabla from the Lua plaintext code which I expect would break if people did local tBla = tracy.Bla and then used tBla(). And with Spring.Bla there is a heavy cargo cult of localizing functions.

[Beherith]

Interesting, what if we just added Spring.LuaTracyPlot to the tracy subtable if tracy is present in system.lua?
So in usage, Spring.LuaTracyPlot would instead be
tracy.LuaTracyPlot()
Would the tracy cleanup stuff also remove this?

[sprunk]

Would the tracy cleanup stuff also remove this?

Yes (it would require a one-liner change in the cleanup func to handle the new var but that's trivial)

[p2004a]

So no, because we don't own cleanup function code, it lives in the tracy repo.

[Beherith]

My preferred solution would still be to have just the bodies of Spring.TracyLuaPlot functions gated within
#if defined(TRACY_ENABLE) blocks, effectively making them near noop's.

1. Since none of the functions have any return values, it doesn't matter to Lua's perspective
2. We don't want to modify tracy upstream, as every time we've done that to a library it comes back later to bite us in the ass when it needs to be updated (remember Assimp, SDL, Lua, etc ), so it cannot be placed in the tracy. namespace.

These plotting functions shouldn't be overused too much anyway, so while 100% free is not possible with them, its only a tiny overhead. There are way more egregious wastes of performance littered everywhere else by game developers compared to some empty function calls that provide highly valuable debugging insight when used.

[sprunk]

We don't want to modify tracy upstream, as every time we've done that to a library it comes back later to bite us in the ass when it needs to be updated (remember Assimp, SDL, Lua, etc )

But none of those were upstreamed, i.e. we never sent patches to the official repos. We just had a patched internal copy of each of those, and that modified copy then got merge conflicts. The idea here would be to add a new Recoil-side function that wraps over tracy's replacer (without touching tracy itself, so no conflicts) and then actually send patches to the tracy repo (which we never did with the earlier libs).

so it cannot be placed in the tracy. namespace.

We can add to it from outside the tracy lib. Consider rts/Lua/LuaMathExtra.cpp which adds to the built-in Lua math. namespace, completely separate from lib/lua/src/lmathlib.cpp which is what creates the built-in math. namespace. Similarly the tracy. Lua namespace can be created and mostly filled inside the tracy lib somewhere in lib/tracy/blabla, and then we would have rts/Lua/LuaTracyExtra.cpp which adds the others.

[p2004a]

+1 to what Sprung said

[sprunk]

I think I can write a PoC of my ideal solution on the weekend

[sprunk]

Here is my PoC implementation: #1651

@p2004a @Beherith

[lhog]

@p2004a can I ask you to decide whether this PR is ready for merge (and press the squash button, if it's)?

[p2004a]

I would like the open thread to be resolved somehow™ before merging.

[sprunk]

Converted to draft to prevent accidental merges because this is generally obsoleted by #1651 unless @Beherith has some feedback.

[Beherith]

Deprecated by 1651