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

Added Trace for the mapping connections, to allow JS profiling. #4766

Merged
merged 4 commits into from
Jun 6, 2022
Merged

Added Trace for the mapping connections, to allow JS profiling. #4766

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ceb69e7
Added Trace for the mapping connections, to allow JS profiling.
JoergAtGithub May 23, 2022
bbc2dba
Merge remote-tracking branch 'upstream/Main' into TraceJavascriptConn…
JoergAtGithub May 24, 2022
0d38845
Move Trace from heap to stack
JoergAtGithub May 25, 2022
ed819cb
Use %1 and argument for string building, to minimize performance impa…
JoergAtGithub May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/controllers/scripting/legacy/scriptconnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
#include "controllers/scripting/legacy/scriptconnection.h"

#include "controllers/scripting/legacy/controllerscriptenginelegacy.h"
#include "util/trace.h"

void ScriptConnection::executeCallback(double value) const {
std::unique_ptr<Trace> pCallCallbackTrace;
pCallCallbackTrace = std::make_unique<Trace>(
QString("JS " + key.item + " callback").toStdString().c_str());
Copy link
Member

Choose a reason for hiding this comment

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

why the unique_ptr? The trace object is not passed around, it could easily be allocated on the stack.
The creation of the QString also involves a heap allocation (likely since the string is likely quite long). toStdString might involves a heap allocation or at least a copy.

Copy link
Member

Choose a reason for hiding this comment

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

The trace Object id designed to live on the stack and will have almost zero overhead in normal mode.
You must not use QString here, because is also allocates memory in non developer mode.

This is the correct usage for minimum over-head at non developer mode:

mixxx/src/soundio/sounddevicenetwork.cpp

Line 415 in d5cf277

Trace trace("SoundDeviceNetwork::callbackProcessClkRef %1",

To speed up the debug case as well, you may consider to introduce a char16_t* overload of Trace and pass the string as u" "
Or even better a template taking char16_t[] to make use of sizeoff() instead of parsing for "/0"

QJSValueList args;
args << QJSValue(value);
args << QJSValue(key.group);
Expand Down