From d8053c9a82c0abcac1740262e862d4f4811b1eb2 Mon Sep 17 00:00:00 2001 From: Maya Lekova Date: Wed, 11 Dec 2019 09:44:53 +0000 Subject: [PATCH] Revert "Support Intel VTune ITT API" This reverts commit 5f5b4b04078a5da96b4c8244241cf73dc928f721. Reason for revert: Breaks vtunejit bot - see https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20vtunejit/32958 Original change's description: > Support Intel VTune ITT API > > Add VTune domain support extension to use VTune Domain/Task API and > tagging trace data for particular JS code block. > > How to use: > 1. Set `"checkout_ittapi" = True` in the custom_vars section of .gclient > file to download intel/ittapi by 'gclient sync' > 2. Build d8 with gn build flag 'v8_enable_vtunetracemark = true' > 3. Run d8 with flag '--enable-vtune-domain-support' > > The Vtune Domain/Task API can be invoked from JS to mark JS code block. > You can mark the start of a JS task by > vtunedomainmark(domain_name, task_name, "start") > and the end of a task by > vtunedomainmark(domain_name, task_name, "end") > Tasks can nest. > > The VTune API (ittapi) is integrated as an external third party library > while the v8_vtune_jit also relies on the VTune ittapi. We have another > patch almost ready which refactors the v8_vtune_jit related code to > depend on the third_party/ittapi. We will submit the refactored v8_vtune_jit > code after this patch stabilized and landed. > > > Contributed by fanchen.kong@intel.com > > Change-Id: I0ecc9dd4e1ea52545f1b6932fcdadfa7c1a6d2b2 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1938490 > Commit-Queue: Shiyu Zhang > Reviewed-by: Hannes Payer > Reviewed-by: Toon Verwaest > Cr-Commit-Position: refs/heads/master@{#65409} TBR=machenbach@chromium.org,hpayer@chromium.org,verwaest@chromium.org,shiyu.zhang@intel.com Change-Id: I44a6e5b1aa32e753ae41966ed321ed787cc752f8 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1960291 Reviewed-by: Maya Lekova Commit-Queue: Maya Lekova Cr-Commit-Position: refs/heads/master@{#65410} --- BUILD.gn | 14 -- DEPS | 8 - .../vtunedomain-support-extension.cc | 144 ------------------ .../vtunedomain-support-extension.h | 47 ------ src/flags/flag-definitions.h | 1 - src/init/bootstrapper.cc | 11 -- src/third_party/vtune/BUILD.gn | 31 ---- src/third_party/vtune/vtuneapi.cc | 83 ---------- src/third_party/vtune/vtuneapi.h | 48 ------ 9 files changed, 387 deletions(-) delete mode 100644 src/extensions/vtunedomain-support-extension.cc delete mode 100644 src/extensions/vtunedomain-support-extension.h delete mode 100644 src/third_party/vtune/vtuneapi.cc delete mode 100644 src/third_party/vtune/vtuneapi.h diff --git a/BUILD.gn b/BUILD.gn index 869e228944..e7a759f9cc 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -75,9 +75,6 @@ declare_args() { # Sets -dENABLE_VTUNE_JIT_INTERFACE. v8_enable_vtunejit = false - # Sets -dENABLE_VTUNE_TRACEMARK. - v8_enable_vtunetracemark = false - # Sets -dENABLE_HANDLE_ZAPPING. v8_enable_handle_zapping = true @@ -408,9 +405,6 @@ config("features") { if (v8_enable_vtunejit) { defines += [ "ENABLE_VTUNE_JIT_INTERFACE" ] } - if (v8_enable_vtunetracemark) { - defines += [ "ENABLE_VTUNE_TRACEMARK" ] - } if (v8_enable_minor_mc) { defines += [ "ENABLE_MINOR_MC" ] } @@ -3331,14 +3325,6 @@ v8_source_set("v8_base_without_compiler") { v8_current_cpu == "s390" || v8_current_cpu == "s390x") { libs += [ "atomic" ] } - - if (v8_enable_vtunetracemark && (is_linux || is_win)) { - sources += [ - "src/extensions/vtunedomain-support-extension.cc", - "src/extensions/vtunedomain-support-extension.h", - ] - deps += [ "src/third_party/vtune:v8_vtune_trace_mark" ] - } } group("v8_base") { diff --git a/DEPS b/DEPS index c2728b59cf..6f0fe5210b 100644 --- a/DEPS +++ b/DEPS @@ -27,7 +27,6 @@ vars = { 'checkout_fuchsia_boot_images': "qemu.x64,qemu.arm64", 'checkout_instrumented_libraries': False, - 'checkout_ittapi': False, 'chromium_url': 'https://chromium.googlesource.com', 'android_url': 'https://android.googlesource.com', 'download_gcmole': False, @@ -259,12 +258,6 @@ deps = { Var('chromium_url') + '/external/github.com/google/protobuf'+ '@' + 'b68a347f56137b4b1a746e8c7438495a6ac1bd91', 'v8/third_party/zlib': Var('chromium_url') + '/chromium/src/third_party/zlib.git'+ '@' + 'f262c1b3c4196a2fee98c113142faff525b8d884', - 'v8/third_party/ittapi': { - # Force checkout ittapi libraries to pass v8 header includes check on - # bots that has check_v8_header_includes enabled. - 'url': Var('chromium_url') + '/external/github.com/intel/ittapi' + '@' + 'b4ae0122ba749163096058b4f1bb065bf4a7de94', - 'condition': "checkout_ittapi or check_v8_header_includes", - }, } include_rules = [ @@ -272,7 +265,6 @@ include_rules = [ '+include', '+unicode', '+third_party/fdlibm', - '+third_party/ittapi/include' ] # checkdeps.py shouldn't check for includes in these directories: diff --git a/src/extensions/vtunedomain-support-extension.cc b/src/extensions/vtunedomain-support-extension.cc deleted file mode 100644 index e5b68ff63d..0000000000 --- a/src/extensions/vtunedomain-support-extension.cc +++ /dev/null @@ -1,144 +0,0 @@ -// Copyright 2019 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "src/extensions/vtunedomain-support-extension.h" -#include -#include - -namespace v8 { -namespace internal { - -namespace libvtune { - -int startTask(const std::vector& vparams); -int endTask(const std::vector& vparams); - -const auto& function_map = - *new std::map&)>{ - {"start", startTask}, {"end", endTask}}; - -void split(const std::string& str, char delimiter, - std::vector* vparams) { - std::string::size_type baseindex = 0; - std::string::size_type offindex = str.find(delimiter); - - while (offindex != std::string::npos) { - (*vparams).push_back(str.substr(baseindex, offindex - baseindex)); - baseindex = ++offindex; - offindex = str.find(delimiter, offindex); - - if (offindex == std::string::npos) - (*vparams).push_back(str.substr(baseindex, str.length())); - } -} - -int startTask(const std::vector& vparams) { - int errcode = 0; - - if (const char* domain_name = vparams[1].c_str()) { - if (const char* task_name = vparams[2].c_str()) { - if (std::shared_ptr domainptr = - VTuneDomain::createDomain(domain_name)) { - if (!domainptr->beginTask(task_name)) { - errcode += TASK_BEGIN_FAILED; - } - } else { - errcode += CREATE_DOMAIN_FAILED; - } - } else { - errcode += NO_TASK_NAME; - } - - } else { - errcode = NO_DOMAIN_NAME; - } - - return errcode; -} - -int endTask(const std::vector& vparams) { - int errcode = 0; - - if (const char* domain_name = vparams[1].c_str()) { - if (std::shared_ptr domainptr = - VTuneDomain::createDomain(domain_name)) { - domainptr->endTask(); - } else { - errcode += CREATE_DOMAIN_FAILED; - } - } else { - errcode = NO_DOMAIN_NAME; - } - - return errcode; -} - -int invoke(const char* params) { - int errcode = 0; - std::vector vparams; - - split(*(new std::string(params)), ' ', &vparams); - - auto it = function_map.find(vparams[0]); - if (it != function_map.end()) { - (it->second)(vparams); - } else { - errcode += UNKNOWN_PARAMS; - } - - return errcode; -} - -} // namespace libvtune - -v8::Local -VTuneDomainSupportExtension::GetNativeFunctionTemplate( - v8::Isolate* isolate, v8::Local str) { - return v8::FunctionTemplate::New(isolate, VTuneDomainSupportExtension::Mark); -} - -// args should take three parameters -// %0 : string, which is the domain name. Domain is used to tagging trace data -// for different modules or libraryies in a program -// %1 : string, which is the task name. Task is a logical unit of work performed -// by a particular thread statement. Task can nest. -// %2 : string, "start" / "end". Action to be taken on a task in a particular -// domain -void VTuneDomainSupportExtension::Mark( - const v8::FunctionCallbackInfo& args) { - if (args.Length() != 3 || !args[0]->IsString() || !args[1]->IsString() || - !args[2]->IsString()) { - args.GetIsolate()->ThrowException( - v8::String::NewFromUtf8( - args.GetIsolate(), - "Parameter number should be exactly three, first domain name" - "second task name, third start/end", - NewStringType::kNormal) - .ToLocalChecked()); - return; - } - - v8::Isolate* isolate = args.GetIsolate(); - v8::String::Utf8Value domainName(isolate, args[0]); - v8::String::Utf8Value taskName(isolate, args[1]); - v8::String::Utf8Value statName(isolate, args[2]); - - char* cdomainName = *domainName; - char* ctaskName = *taskName; - char* cstatName = *statName; - - std::stringstream params; - params << cstatName << " " << cdomainName << " " << ctaskName; - - int r = 0; - if ((r = libvtune::invoke(params.str().c_str())) != 0) { - args.GetIsolate()->ThrowException( - v8::String::NewFromUtf8(args.GetIsolate(), std::to_string(r).c_str(), - NewStringType::kNormal) - .ToLocalChecked()); - } -} - -} // namespace internal -} // namespace v8 diff --git a/src/extensions/vtunedomain-support-extension.h b/src/extensions/vtunedomain-support-extension.h deleted file mode 100644 index 394cc3fb21..0000000000 --- a/src/extensions/vtunedomain-support-extension.h +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2019 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef V8_EXTENSIONS_VTUNEDOMAIN_SUPPORT_EXTENSION_H_ -#define V8_EXTENSIONS_VTUNEDOMAIN_SUPPORT_EXTENSION_H_ - -#include "include/v8.h" -#include "src/third_party/vtune/vtuneapi.h" -#include "src/utils/utils.h" - -#define UNKNOWN_PARAMS 1 << 0 -#define NO_DOMAIN_NAME 1 << 1 -#define CREATE_DOMAIN_FAILED 1 << 2 -#define NO_TASK_NAME 1 << 3 -#define CREATE_TASK_FAILED 1 << 4 -#define TASK_BEGIN_FAILED 1 << 5 -#define TASK_END_FAILED 1 << 6 - -namespace v8 { -namespace internal { - -class VTuneDomainSupportExtension : public v8::Extension { - public: - explicit VTuneDomainSupportExtension(const char* fun_name = "test") - : v8::Extension("v8/vtunedomain", - BuildSource(buffer_, sizeof(buffer_), fun_name)) {} - - v8::Local GetNativeFunctionTemplate( - v8::Isolate* isolate, v8::Local name) override; - - private: - static void Mark(const v8::FunctionCallbackInfo& args); - - static const char* BuildSource(char* buf, size_t size, const char* fun_name) { - SNPrintF(Vector(buf, static_cast(size)), "native function %s();", - fun_name); - return buf; - } - - char buffer_[50]; -}; - -} // namespace internal -} // namespace v8 - -#endif // V8_EXTENSIONS_VTUNEDOMAIN_SUPPORT_EXTENSION_H_ diff --git a/src/flags/flag-definitions.h b/src/flags/flag-definitions.h index bef4e448a2..1430de7de1 100644 --- a/src/flags/flag-definitions.h +++ b/src/flags/flag-definitions.h @@ -1075,7 +1075,6 @@ DEFINE_BOOL(disallow_code_generation_from_strings, false, DEFINE_BOOL(expose_async_hooks, false, "expose async_hooks object") DEFINE_STRING(expose_cputracemark_as, nullptr, "expose cputracemark extension under the specified name") -DEFINE_BOOL(enable_vtune_domain_support, false, "enable vtune domain support") // builtins.cc DEFINE_BOOL(allow_unsafe_function_constructor, false, diff --git a/src/init/bootstrapper.cc b/src/init/bootstrapper.cc index 1fbb43a986..ef0b5d4566 100644 --- a/src/init/bootstrapper.cc +++ b/src/init/bootstrapper.cc @@ -19,9 +19,6 @@ #include "src/extensions/ignition-statistics-extension.h" #include "src/extensions/statistics-extension.h" #include "src/extensions/trigger-failure-extension.h" -#ifdef ENABLE_VTUNE_TRACEMARK -#include "src/extensions/vtunedomain-support-extension.h" -#endif // ENABLE_VTUNE_TRACEMARK #include "src/heap/heap-inl.h" #include "src/logging/counters.h" #include "src/numbers/math-random.h" @@ -133,10 +130,6 @@ void Bootstrapper::InitializeOncePerProcess() { v8::RegisterExtension( std::make_unique(FLAG_expose_cputracemark_as)); } -#ifdef ENABLE_VTUNE_TRACEMARK - v8::RegisterExtension( - std::make_unique("vtunedomainmark")); -#endif // ENABLE_VTUNE_TRACEMARK } void Bootstrapper::TearDown() { @@ -5039,10 +5032,6 @@ bool Genesis::InstallExtensions(Isolate* isolate, &extension_states)) && (!isValidCpuTraceMarkFunctionName() || InstallExtension(isolate, "v8/cpumark", &extension_states)) && -#ifdef ENABLE_VTUNE_TRACEMARK - (!FLAG_enable_vtune_domain_support || - InstallExtension(isolate, "v8/vtunedomain", &extension_states)) && -#endif // ENABLE_VTUNE_TRACEMARK InstallRequestedExtensions(isolate, extensions, &extension_states); } diff --git a/src/third_party/vtune/BUILD.gn b/src/third_party/vtune/BUILD.gn index fe289ad7c3..a2163ebaf1 100644 --- a/src/third_party/vtune/BUILD.gn +++ b/src/third_party/vtune/BUILD.gn @@ -4,13 +4,6 @@ import("//build/toolchain/toolchain.gni") -config("vtune_ittapi") { - include_dirs = [ - "../../../third_party/ittapi/include", - "../../../third_party/ittapi/src/ittnotify", - ] -} - static_library("v8_vtune") { sources = [ "ittnotify_config.h", @@ -25,27 +18,3 @@ static_library("v8_vtune") { "../../..:v8", ] } - -static_library("v8_vtune_trace_mark") { - sources = [ - "../../../third_party/ittapi/include/ittnotify.h", - "vtuneapi.cc", - "vtuneapi.h", - ] - deps = [ - ":ittnotify", - ] -} - -static_library("ittnotify") { - sources = [ - "../../../third_party/ittapi/include/ittnotify.h", - "../../../third_party/ittapi/include/legacy/ittnotify.h", - "../../../third_party/ittapi/src/ittnotify/disable-warnings.h", - "../../../third_party/ittapi/src/ittnotify/ittnotify_config.h", - "../../../third_party/ittapi/src/ittnotify/ittnotify_static.c", - "../../../third_party/ittapi/src/ittnotify/ittnotify_static.h", - ] - include_dirs = [ "../../../third_party/ittapi/include" ] - deps = [] -} diff --git a/src/third_party/vtune/vtuneapi.cc b/src/third_party/vtune/vtuneapi.cc deleted file mode 100644 index b6f7b73d59..0000000000 --- a/src/third_party/vtune/vtuneapi.cc +++ /dev/null @@ -1,83 +0,0 @@ -#include "vtuneapi.h" -#ifdef _MSC_VER // for msvc -#include -#endif - -std::map> VTuneDomain::domains_; -std::map VTuneDomain::string_handlers_; - -std::shared_ptr VTuneDomain::createDomain( - const char* domain_name) { - auto domain = getDomain(domain_name); - - if (domain == nullptr) { -#ifdef _MSC_VER // for msvc - wchar_t buffer[255]; - mbstowcs(buffer, domain_name, 255); - __itt_domain* itt_domain = __itt_domain_create(buffer); // call api -#else // for clang and gcc - __itt_domain* itt_domain = __itt_domain_create(domain_name); // call api -#endif - if (itt_domain != NULL) { - std::string key(domain_name); - std::shared_ptr value(new VTuneDomain(itt_domain)); - domain = value; - domains_.insert(std::make_pair(key, value)); - } - } - - return domain; -} - -void VTuneDomain::destroyDomain(const char* domain_name) { - auto it = domains_.find(domain_name); - if (it != domains_.end()) { - domains_.erase(it); - } -} - -std::shared_ptr VTuneDomain::getDomain(const char* domain_name) { - std::shared_ptr result(nullptr); - - auto it = domains_.find(domain_name); - if (it != domains_.end()) { - result = it->second; - } - - return result; -} - -__itt_string_handle* VTuneDomain::getString(const char* str) { - __itt_string_handle* result = NULL; - - auto it = string_handlers_.find(str); - if (it != string_handlers_.end()) { - result = it->second; - } else { -#ifdef _MSC_VER // for msvc - wchar_t buffer[255]; - mbstowcs(buffer, str, 255); - result = __itt_string_handle_create(buffer); // call api -#else // for clang and gcc - result = __itt_string_handle_create(str); -#endif - std::string key(str); - string_handlers_.insert(std::make_pair(key, result)); - } - - return result; -} - -bool VTuneDomain::beginTask(const char* task_name) { - bool result = false; - - __itt_string_handle* name = getString(task_name); - if (name != NULL) { - __itt_task_begin(domain_, __itt_null, __itt_null, name); - result = true; - } - - return result; -} - -void VTuneDomain::endTask() { __itt_task_end(domain_); } diff --git a/src/third_party/vtune/vtuneapi.h b/src/third_party/vtune/vtuneapi.h deleted file mode 100644 index ee3addd943..0000000000 --- a/src/third_party/vtune/vtuneapi.h +++ /dev/null @@ -1,48 +0,0 @@ -#ifndef __VTUNE_API_H__ -#define __VTUNE_API_H__ - -#include -#include -#include -#include "third_party/ittapi/include/ittnotify.h" - -class VTuneDomain { - /* factory method to create vtune domain */ - public: - /* create vtune domain, domain_name should be unique, if there - already has a domain with same name, return false. - */ - static std::shared_ptr createDomain(const char* domain_name); - - /* destory vtune domain*/ - static void destroyDomain(const char* domain_name); - - /* get a domain */ - static std::shared_ptr getDomain(const char* domain_name); - - /* domain interface */ - public: - /* start a task with name, if no task name or null, use "default_task_name"*/ - bool beginTask(const char* task_name = "default_task_name"); - - /* end a task, if there is no stacked task, return false*/ - void endTask(); - - public: - virtual ~VTuneDomain() {} - - protected: - static __itt_string_handle* getString(const char* str); - - protected: - VTuneDomain(__itt_domain* domain) { domain_ = domain; } - - private: - static std::map> domains_; - static std::map string_handlers_; - - private: - __itt_domain* domain_; -}; - -#endif