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

WIP: COMMON: Added tracy profiler support #2686

Closed
wants to merge 1 commit into from
Closed

WIP: COMMON: Added tracy profiler support #2686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
View file
Open in desktop
Expand Up @@ -2,3 +2,6 @@
path = devtools/create_prince
url = https://github.com/scummvm/game-translations
branch = prince-and-the-coward
[submodule "common/tracy"]
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of throwing in code to the codebase like this. Would like to hear opinions of other devs.

I see that the lib author made it very straightforward by including everything into once source file, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely true - this is just for letting people play with it to decide whether a proper integration is worth the work. Just like to get feedback if I should continue to integrate it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "problem" that I see here is that it's not a single c++ source file - but a lot of different headers and a lot of c++ files. Also the capture files are save, but the network protocol might differ from version to version. So it's the best to keep the profiler ui and the profiler "server" in sync somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The author is suggesting this as integration method, too - https://github.com/wolfpld/tracy/blob/master/manual/tracy.tex#L388

One could also just assume that people who want to profile can copy the source files manually to the proper location and the build system will pick it up if that files exist. This wouldn't be a problem for make - but for msvc you would most likely have to re-run create_project for it.

path = common/tracy
url = https://github.com/wolfpld/tracy.git
2 changes: 2 additions & 0 deletions backends/graphics/openglsdl/openglsdl-graphics.cpp
View file
Open in desktop
Expand Up @@ -24,6 +24,7 @@
#include "backends/graphics/opengl/texture.h"
#include "backends/events/sdl/sdl-events.h"
#include "backends/platform/sdl/sdl.h"
#include "common/trace.h"
#include "graphics/scaler/aspect.h"

#include "common/textconsole.h"
Expand Down Expand Up @@ -341,6 +342,7 @@ void OpenGLSdlGraphicsManager::refreshScreen() {
#else
SDL_GL_SwapBuffers();
#endif
traceEndFrame(refreshScreen);
}

void *OpenGLSdlGraphicsManager::getProcAddress(const char *name) const {
Expand Down
2 changes: 2 additions & 0 deletions backends/graphics/surfacesdl/surfacesdl-graphics.cpp
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@
*/

#include "common/scummsys.h"
#include "common/trace.h"

#if defined(SDL_BACKEND)
#include "backends/graphics/surfacesdl/surfacesdl-graphics.h"
Expand Down Expand Up @@ -1396,6 +1397,7 @@ void SurfaceSdlGraphicsManager::internUpdateScreen() {
_numDirtyRects = 0;
_forceRedraw = false;
_cursorNeedsRedraw = false;
traceEndFrame(updateScreen);
}

bool SurfaceSdlGraphicsManager::saveScreenshot(const Common::String &filename) const {
Expand Down
3 changes: 3 additions & 0 deletions backends/graphics3d/openglsdl/openglsdl-graphics3d.cpp
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@
#include "backends/events/sdl/sdl-events.h"
#include "common/config-manager.h"
#include "common/file.h"
#include "common/trace.h"
#include "engines/engine.h"
#include "graphics/pixelbuffer.h"
#include "graphics/opengl/context.h"
Expand Down Expand Up @@ -573,6 +574,8 @@ void OpenGLSdlGraphics3dManager::updateScreen() {
SDL_GL_SwapBuffers();
#endif

traceEndFrame(updateScreen);

if (_frameBuffer) {
_frameBuffer->attach();
}
Expand Down
5 changes: 5 additions & 0 deletions common/module.mk
View file
Open in desktop
Expand Up @@ -60,6 +60,11 @@ MODULE_OBJS += \
rdft.o \
sinetables.o

ifdef ENABLE_TRACY
MODULE_OBJS += \
tracy/TracyClient.o
endif

ifdef ENABLE_EVENTRECORDER
MODULE_OBJS += \
recorderfile.o
Expand Down
52 changes: 52 additions & 0 deletions common/trace.h
View file
Open in desktop
@@ -0,0 +1,52 @@
/* ScummVM - Graphic Adventure Engine
*
* ScummVM is the legal property of its developers, whose names
* are too numerous to list here. Please refer to the COPYRIGHT
* file distributed with this source distribution.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/

#ifndef COMMON_TRACE_H
#define COMMON_TRACE_H

#ifdef TRACY_ENABLE
#include "tracy/Tracy.hpp"

#define traceValueScoped(name, x) ZoneNamedN(__tracy_scoped_##name, #name, true); ZoneValueV(__tracy_scoped_##name, (uint64_t)(x))
#define tracePlot(name, x) TracyPlot(name, x)
#define traceMsg(message) TracyMessageL(message)
#define traceThread(name) tracy::SetThreadName(name)
#define traceMutex(type, varname, name) type varname { tracy::SourceLocationData{ nullptr, name, __FILE__, __LINE__, 0 } }

#define traceBeginFrame(name) (void)0
#define traceEndFrame(name) FrameMark
#define traceScoped(name) ZoneNamedN(__tracy_scoped_##name, #name, true)
#define traceScopedFunc() ZoneNamedN(__tracy_scoped_function_, __FUNCTION__, true)
#else
#define traceValueScoped(name, x) (void)0
#define tracePlot(name, x) (void)0
#define traceMsg(message) (void)0
#define traceThread(name) (void)0
#define traceMutex(type, varname, name) type varname

#define traceBeginFrame(name) (void)0
#define traceEndFrame(name) (void)0
#define traceScoped(name) (void)0
#define traceScopedFunc() (void)0
#endif

#endif
1 change: 1 addition & 0 deletions common/tracy
Open in desktop
Submodule tracy added at 0325f8
20 changes: 20 additions & 0 deletions configure
View file
Open in desktop
Expand Up @@ -199,6 +199,8 @@ _pandoc=no
# Default vkeybd/eventrec options
_vkeybd=no
_eventrec=no
# Tracy Profiler support
_tracy=no
# GUI translation options
_translation=yes
# Default platform settings
Expand Down Expand Up @@ -1063,6 +1065,7 @@ Optional Features:
--enable-vkeybd build virtual keyboard support
--enable-eventrecorder enable event recording functionality
--disable-eventrecorder disable event recording functionality
--enable-tracy enable tracy profiler support
--enable-updates build support for updates
--enable-text-console use text console instead of graphical console
--enable-verbose-build enable regular echoing of commands during build
Expand Down Expand Up @@ -1346,6 +1349,7 @@ for ac_option in $@; do
--disable-vkeybd) _vkeybd=no ;;
--enable-eventrecorder) _eventrec=yes ;;
--disable-eventrecorder) _eventrec=no ;;
--enable-tracy) _tracy=yes ;;
--enable-text-console) _text_console=yes ;;
--disable-text-console) _text_console=no ;;
--with-fluidsynth-prefix=*)
Expand Down Expand Up @@ -5689,6 +5693,18 @@ echo "$_discord"
define_in_config_if_yes $_vkeybd 'ENABLE_VKEYBD'
define_in_config_if_yes $_eventrec 'ENABLE_EVENTRECORDER'

#
# Check whether to enable tracy profiler support
#

if test "$_tracy" = yes ; then
append_var DEFINES "-DTRACY_ENABLE"
append_var LIBS "-ldl -lpthread"
define_in_config_if_yes $_tracy 'FORBIDDEN_SYMBOL_ALLOW_ALL'
fi
define_in_config_if_yes $_tracy 'ENABLE_TRACY'
define_in_config_if_yes $_tracy 'TRACY_ON_DEMAND'

# Check whether to build translation support
#
echo_n "Building translation support... "
Expand Down Expand Up @@ -5981,6 +5997,10 @@ if test "$_eventrec" = yes ; then
echo_n ", event recorder"
fi

if test "$_tracy" = yes ; then
echo_n ", tracy profiler"
fi

if test "$_cloud" = yes ; then
echo ", cloud"
else
Expand Down
1 change: 1 addition & 0 deletions devtools/create_project/create_project.cpp
View file
Open in desktop
Expand Up @@ -1070,6 +1070,7 @@ const Feature s_features[] = {
{ "translation", "USE_TRANSLATION", false, true, "Translation support" },
{ "vkeybd", "ENABLE_VKEYBD", false, false, "Virtual keyboard support"},
{ "eventrecorder", "ENABLE_EVENTRECORDER", false, false, "Event recorder support"},
{ "tracy", "ENABLE_TRACY", false, false, "Tracy profiler support"},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also not yet done - will be finished if we can agree to include this - but TRACY_ENABLE must be given as define. That is not yet done here.

{ "updates", "USE_UPDATES", false, false, "Updates support"},
{ "dialogs", "USE_SYSDIALOGS", false, true, "System dialogs support"},
{ "langdetect", "USE_DETECTLANG", false, true, "System language detection support" }, // This feature actually depends on "translation", there
Expand Down