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

Add support for emitting ETW events to LTTng #4314

Merged
merged 3 commits into from
Dec 7, 2017
Merged

Add support for emitting ETW events to LTTng #4314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98dc145
Tweaking ETW manifest for easier auto-generation of LTTng bindings
MSLaguana Nov 22, 2017
6756e7d
Enable support for ETW events on xplat via LTTng
MSLaguana Nov 22, 2017
41d2de9
Moving custom ETW event firing out of windows specifc code
MSLaguana Nov 29, 2017
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
16 changes: 16 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,5 +495,21 @@ add_definitions(
-DNO_PAL_MINMAX
-DPAL_STDCPP_COMPAT
)

if (ENABLE_JS_LTTNG_SH)
unset(ENABLE_JS_LTTNG_SH CACHE)
include_directories (
${CMAKE_CURRENT_SOURCE_DIR}/out/lttng
)
add_subdirectory ($ENV{TARGET_PATH}/lttng ${CMAKE_CURRENT_BINARY_DIR}/lttng)

add_definitions(
-DENABLE_JS_ETW
-DENABLE_JS_LTTNG
)
set(USE_LTTNG "1")
endif()

add_subdirectory (lib)

add_subdirectory (bin)
2 changes: 1 addition & 1 deletion bin/GCStress/StubExternalApi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void ConfigParserAPI::DisplayInitialOutput(__in LPWSTR moduleName)
Output::Print(_u("INIT: DLL Path : %s\n"), moduleName);
}

#ifdef ENABLE_JS_ETW
#if defined(ENABLE_JS_ETW) && !defined(ENABLE_JS_LTTNG)
void EtwCallbackApi::OnSessionChange(ULONG /* controlCode */, PVOID /* callbackContext */)
{
// Does nothing
Expand Down
18 changes: 17 additions & 1 deletion build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ PRINT_USAGE() {
echo " --libs-only Do not build CH and GCStress"
echo " --lto Enables LLVM Full LTO"
echo " --lto-thin Enables LLVM Thin LTO - xcode 8+ or clang 3.9+"
echo " --lttng Enables LTTng support for ETW events"
Copy link
Collaborator

Choose a reason for hiding this comment

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

show error / warning if it's used on OSX? or add a note here?

echo " --static Build as static library. Default: shared library"
echo " --sanitize=CHECKS Build with clang -fsanitize checks,"
echo " e.g. undefined,signed-integer-overflow."
Expand Down Expand Up @@ -104,6 +105,7 @@ OS_LINUX=0
OS_APT_GET=0
OS_UNIX=0
LTO=""
LTTNG=""
TARGET_OS=""
ENABLE_CC_XPLAT_TRACE=""
WB_CHECK=
Expand Down Expand Up @@ -230,6 +232,11 @@ while [[ $# -gt 0 ]]; do
HAS_LTO=1
;;

--lttng)
LTTNG="-DENABLE_JS_LTTNG_SH=1"
HAS_LTTNG=1
;;

-n | --ninja)
CMAKE_GEN="-G Ninja"
MAKE=ninja
Expand Down Expand Up @@ -516,6 +523,15 @@ else
exit 1
fi
fi
export TARGET_PATH

if [[ $HAS_LTTNG == 1 ]]; then
CHAKRACORE_ROOT=`dirname $0`
python $CHAKRACORE_ROOT/tools/lttng.py --man $CHAKRACORE_ROOT/manifests/Microsoft-Scripting-Chakra-Instrumentation.man --intermediate $TARGET_PATH/intermediate
mkdir -p $TARGET_PATH/lttng
(diff -q $TARGET_PATH/intermediate/lttng/jscriptEtw.h $TARGET_PATH/lttng/jscriptEtw.h && echo "jscriptEtw.h up to date; skipping") || cp $TARGET_PATH/intermediate/lttng/* $TARGET_PATH/lttng/
fi


BUILD_DIRECTORY="${TARGET_PATH}/${BUILD_TYPE:0}"
echo "Build path: ${BUILD_DIRECTORY}"
Expand Down Expand Up @@ -602,7 +618,7 @@ fi

echo Generating $BUILD_TYPE makefiles
echo $EXTRA_DEFINES
cmake $CMAKE_GEN $CC_PREFIX $ICU_PATH $LTO $STATIC_LIBRARY $ARCH $TARGET_OS \
cmake $CMAKE_GEN $CC_PREFIX $ICU_PATH $LTO $LTTNG $STATIC_LIBRARY $ARCH $TARGET_OS \
$ENABLE_CC_XPLAT_TRACE $EXTRA_DEFINES -DCMAKE_BUILD_TYPE=$BUILD_TYPE $SANITIZE $NO_JIT $INTL_ICU \
$WITHOUT_FEATURES $WB_FLAG $WB_ARGS $CMAKE_EXPORT_COMPILE_COMMANDS $LIBS_ONLY_BUILD\
$VALGRIND $BUILD_RELATIVE_DIRECTORY
Expand Down
12 changes: 12 additions & 0 deletions lib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ if(CAN_BUILD_WABT)
set(wabt_includes ${CHAKRACORE_SOURCE_DIR}/lib/wabt)
endif()

if (USE_LTTNG)
set(lttng_objects $<TARGET_OBJECTS:Chakra.LTTng>)
endif()

add_library (ChakraCoreStatic STATIC
ChakraCoreStatic.cpp
$<TARGET_OBJECTS:Chakra.Pal>
Expand All @@ -38,8 +42,16 @@ add_library (ChakraCoreStatic STATIC
$<TARGET_OBJECTS:Chakra.Parser>
${wasm_objects}
${wabt_objects}
${lttng_objects}
)

if(USE_LTTNG)
Copy link
Collaborator

Choose a reason for hiding this comment

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

lttng is not for osx ++ we better share target_link_libs below.. So, move this under else() for linux below and define ${LTTNG....} and use it under the same target_link_libraries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having multiple target_link_libraries is fine and additive so that shouldn't cause issues. Since lttng is opt-in, I was thinking that on mac people would just not specify the --lttng flag (or if they did and it broke, they would see that lttng wasn't supported on their system). Do you feel strongly about this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be nice to combine them in the same place in order to prevent future what is happening here confusions.

target_link_libraries(ChakraCoreStatic
-llttng-ust
-ldl
)
endif()

if(CC_TARGET_OS_OSX)
target_link_libraries(ChakraCoreStatic
"-framework CoreFoundation"
Expand Down
4 changes: 3 additions & 1 deletion lib/Common/Core/EtwTraceCore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "Core/EtwTraceCore.h"

#ifdef ENABLE_JS_ETW
#ifndef ENABLE_JS_LTTNG
extern "C" {
ETW_INLINE
VOID EtwCallback(
Expand Down Expand Up @@ -64,4 +65,5 @@ void EtwTraceCore::UnRegister()
}
}

#endif
#endif // !ENABLE_JS_LTTNG
#endif // ENABLE_JS_ETW
5 changes: 5 additions & 0 deletions lib/Common/Core/EtwTraceCore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
#define JS_ETW(s) s
#define IS_JS_ETW(s) s

#ifdef ENABLE_JS_LTTNG
#include "jscriptEtw.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is somewhat confusing here (if lttng { include etw }) -- can the output file be jscriptLTTng.h instead? (Or, even better, JScriptLTTng.h or JScriptLttng.h

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 jscriptEtw.h file is kind of the cross-over point between ETW and our LTTng support: it provides an ETW-like interface (e.g. EmitEvent* functions) that call into our LTTng functions. If you still think it is confusing, I don't mind changing the name.


#else
// C-style callback
extern "C" {
void EtwCallback(
Expand Down Expand Up @@ -87,6 +91,7 @@ class EtwTraceCore

static bool s_registered;
};
#endif // ENABLE_JS_LTTNG

#else
#define GCETW(e, ...)
Expand Down
8 changes: 5 additions & 3 deletions lib/Common/Memory/Recycler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5968,7 +5968,8 @@ Recycler::ThreadProc()
}
#endif

#ifdef ENABLE_JS_ETW
#if defined(ENABLE_JS_ETW) && ! defined(ENABLE_JS_LTTNG)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because LTTng doesn't have any concept of EventActivityIdControl

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps call that out as the reason why, here?

// LTTng has no concept of EventActivityIdControl
// Create an ETW ActivityId for this thread, to help tools correlate ETW events we generate
GUID activityId = { 0 };
auto eventActivityIdControlResult = EventActivityIdControl(EVENT_ACTIVITY_CTRL_CREATE_SET_ID, &activityId);
Expand Down Expand Up @@ -6536,7 +6537,8 @@ RecyclerParallelThread::StaticThreadProc(LPVOID lpParameter)
dllHandle = NULL;
}
#endif
#ifdef ENABLE_JS_ETW
#if defined(ENABLE_JS_ETW) && ! defined(ENABLE_JS_LTTNG)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

// LTTng has no concept of EventActivityIdControl
// Create an ETW ActivityId for this thread, to help tools correlate ETW events we generate
GUID activityId = { 0 };
auto eventActivityIdControlResult = EventActivityIdControl(EVENT_ACTIVITY_CTRL_CREATE_SET_ID, &activityId);
Expand Down Expand Up @@ -8757,4 +8759,4 @@ template char* Recycler::AllocZeroWithAttributesInlined<RecyclerVisitedHostTrace
template char* Recycler::AllocZeroWithAttributesInlined<RecyclerVisitedHostFinalizableBits, /* nothrow = */true>(size_t);
template char* Recycler::AllocZeroWithAttributesInlined<RecyclerVisitedHostTracedBits, /* nothrow = */true>(size_t);
template char* Recycler::AllocZeroWithAttributesInlined<LeafBit, /* nothrow = */true>(size_t);
#endif
#endif
2 changes: 1 addition & 1 deletion lib/Jsrt/JsrtHelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ void JsrtCallbackState::ObjectBeforeCallectCallbackWrapper(JsObjectBeforeCollect
ConfigParser::ParseOnModuleLoad(parser, mod);
}

#ifdef ENABLE_JS_ETW
#if defined(ENABLE_JS_ETW) && !defined(ENABLE_JS_LTTNG)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LTTng doesn't have the same register/unregister hooks that ETW has, so this is irrelevant for LTTng, and I thought it was simpler to just remove/avoid the whole registration concept.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We might implement dummy functions instead?

Why -> Adding an additional platform comes with an additional set of dependencies / call requirements. It could be nice to have a common interface (even if it's a dummy function)

EtwTrace::Register();
#endif
#ifdef VTUNE_PROFILING
Expand Down
4 changes: 2 additions & 2 deletions lib/Jsrt/JsrtSourceHolder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ namespace Js

#if ENABLE_DEBUG_CONFIG_OPTIONS
AssertMsg(reasonString != nullptr, "Reason string for why we are mapping the source was not provided.");
JS_ETW(EventWriteJSCRIPT_SOURCEMAPPING((uint32)wcslen(reasonString), reasonString, (ushort)requestedFor));
JS_ETW(EventWriteJSCRIPT_SOURCEMAPPING(reasonString, (ushort)requestedFor));
#endif
}

Expand Down Expand Up @@ -285,7 +285,7 @@ namespace Js

#if ENABLE_DEBUG_CONFIG_OPTIONS
AssertMsg(reasonString != nullptr, "Reason string for why we are mapping the source was not provided.");
JS_ETW(EventWriteJSCRIPT_SOURCEMAPPING((uint32)wcslen(reasonString), reasonString, (ushort)requestedFor));
JS_ETW(EventWriteJSCRIPT_SOURCEMAPPING(reasonString, (ushort)requestedFor));
#endif
}

Expand Down
8 changes: 7 additions & 1 deletion lib/Runtime/Base/EtwTrace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

using namespace Js;

#ifndef ENABLE_JS_LTTNG
//
// This C style callback is invoked by ETW when a trace session is started/stopped
// by an ETW controller for the Jscript and MSHTML providers.
Expand Down Expand Up @@ -47,14 +48,17 @@ void EtwCallbackApi::OnSessionChange(ULONG controlCode, PVOID callbackContext)
}
}
}
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: could be nice to add // ENABLE_JS_LTTNG at the end of #endif


//
// Registers the ETW provider - this is usually done on Jscript DLL load
// After registration, we will receive callbacks when ETW tracing is enabled/disabled.
//
void EtwTrace::Register()
{
#ifndef ENABLE_JS_LTTNG
EtwTraceCore::Register();
#endif

#ifdef TEST_ETW_EVENTS
TestEtwEventSink::Load();
Expand All @@ -66,8 +70,10 @@ void EtwTrace::Register()
//
void EtwTrace::UnRegister()
{
#ifndef ENABLE_JS_LTTNG
EtwTraceCore::UnRegister();

#endif

#ifdef TEST_ETW_EVENTS
TestEtwEventSink::Unload();
#endif
Expand Down
4 changes: 2 additions & 2 deletions lib/Runtime/Base/ScriptContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6159,11 +6159,11 @@ void ScriptContext::RegisterPrototypeChainEnsuredToHaveOnlyWritableDataPropertie

if (emitV2AsyncStackEvent)
{
JS_ETW(EventWriteJSCRIPT_ASYNCCAUSALITY_STACKTRACE_V2(operationID, frameCount, nameBufferLength, sizeof(StackFrameInfo), &stackFrames.Item(0), nameBufferString));
JS_ETW(EventWriteJSCRIPT_ASYNCCAUSALITY_STACKTRACE_V2(operationID, frameCount, nameBufferLength, nameBufferString, sizeof(StackFrameInfo), &stackFrames.Item(0)));
}
else
{
JS_ETW(EventWriteJSCRIPT_STACKTRACE(operationID, frameCount, nameBufferLength, sizeof(StackFrameInfo), &stackFrames.Item(0), nameBufferString));
JS_ETW(EventWriteJSCRIPT_STACKTRACE(operationID, frameCount, nameBufferLength, nameBufferString, sizeof(StackFrameInfo), &stackFrames.Item(0)));
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions lib/Runtime/Library/GlobalObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,8 +1621,7 @@ namespace Js
return function->GetScriptContext()->GetLibrary()->GetUndefined();
}

Js::JavascriptString* jsString = Js::JavascriptConversion::ToString(args[1], function->GetScriptContext());
PlatformAgnostic::EventTrace::FireGenericEventTrace(jsString->GetSz());
JS_ETW(EventWriteJSCRIPT_INTERNAL_GENERIC_EVENT(Js::JavascriptConversion::ToString(args[1], function->GetScriptContext())->GetSz()));
return function->GetScriptContext()->GetLibrary()->GetUndefined();
}
#endif
Expand Down
2 changes: 0 additions & 2 deletions lib/Runtime/PlatformAgnostic/Chakra.Runtime.PlatformAgnostic.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,13 @@
<ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\SystemInfo.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\Thread.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Platform\Common\UnicodeText.Common.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\EventTrace.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)Platform\Windows\PerfTrace.cpp" />
</ItemGroup>
<ItemGroup Condition="'$(IntlICU)'=='true'">
<ClCompile Include="$(MSBuildThisFileDirectory)Platform\Common\Intl.cpp" />
</ItemGroup>
<ItemGroup>
<ClInclude Include="ChakraPlatform.h" />
<ClInclude Include="EventTrace.h" />
<ClInclude Include="PerfTrace.h" />
<ClInclude Include="RuntimePlatformAgnosticPch.h" />
<ClInclude Include="UnicodeText.h" />
Expand Down
10 changes: 2 additions & 8 deletions lib/Runtime/PlatformAgnostic/Chakra.Runtime.PlatformAgnostic.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<ItemGroup>
<ClCompile Include="Platform\Windows\UnicodeText.cpp">
<Filter>Platform\Windows</Filter>
</ClCompile>
<ClCompile Include="Platform\Windows\EventTrace.cpp">
<Filter>Platform\Windows</Filter>
</ClCompile>
<ClCompile Include="Platform\Windows\DaylightHelper.cpp">
<Filter>Platform\Windows</Filter>
</ClCompile>
Expand Down Expand Up @@ -51,14 +48,11 @@
</ClInclude>
<ClInclude Include="ChakraPlatform.h" />
<ClInclude Include="RuntimePlatformAgnosticPch.h" />
<ClInclude Include="EventTrace.h">
<Filter>Interfaces</Filter>
</ClInclude>
<ClInclude Include="PerfTrace.h">
<Filter>Interfaces</Filter>
</ClInclude>
<ClInclude Include="IPlatformAgnosticResource.h">
<Filter>Interfaces</Filter>
</ClInclude>
</ItemGroup>
</Project>
</Project>
1 change: 0 additions & 1 deletion lib/Runtime/PlatformAgnostic/ChakraPlatform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#pragma once

#include "UnicodeText.h"
#include "EventTrace.h"

// Configure whether we configure a signal handler
// to produce perf-<pid>.map files
Expand Down
15 changes: 0 additions & 15 deletions lib/Runtime/PlatformAgnostic/EventTrace.h
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion lib/Runtime/PlatformAgnostic/Platform/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ set(PL_SOURCE_FILES
Common/UnicodeText.Common.cpp
Common/HiResTimer.cpp
Common/DateTime.cpp
Linux/EventTrace.cpp
Linux/UnicodeText.ICU.cpp
Linux/NumbersUtility.cpp
Linux/Thread.cpp
Expand Down
18 changes: 0 additions & 18 deletions lib/Runtime/PlatformAgnostic/Platform/Linux/EventTrace.cpp
View file
Open in desktop

This file was deleted.

19 changes: 0 additions & 19 deletions lib/Runtime/PlatformAgnostic/Platform/Windows/EventTrace.cpp
View file
Open in desktop

This file was deleted.

Loading