From 5b19cbce34b0999b28d0bf3cb2878dc17cdd81f1 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Mon, 31 Mar 2025 11:35:01 +0200 Subject: [PATCH 1/3] Revert "More prohibition of fp register use (#194)" This reverts commit 601d2a14deba6f7ca58633d3391480b665a6b5b3. --- bindings/translate-heap-profile.cc | 8 +++----- bindings/translate-time-profile.cc | 20 +++++++++----------- bindings/translate-time-profile.hh | 3 +-- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/bindings/translate-heap-profile.cc b/bindings/translate-heap-profile.cc index b3eed7bc..510bd8c7 100644 --- a/bindings/translate-heap-profile.cc +++ b/bindings/translate-heap-profile.cc @@ -41,7 +41,7 @@ class HeapProfileTranslator : ProfileTranslator { public: v8::Local TranslateAllocationProfile( - v8::AllocationProfile::Node* node) GENERAL_REGS_ONLY { + v8::AllocationProfile::Node* node) { v8::Local children = NewArray(node->children.size()); for (size_t i = 0; i < node->children.size(); i++) { Set(children, i, TranslateAllocationProfile(node->children[i])); @@ -71,8 +71,7 @@ class HeapProfileTranslator : ProfileTranslator { v8::Local lineNumber, v8::Local columnNumber, v8::Local children, - v8::Local allocations) - GENERAL_REGS_ONLY { + v8::Local allocations) { v8::Local js_node = NewObject(); #define X(name) Set(js_node, str_##name, name); NODE_FIELDS @@ -82,8 +81,7 @@ class HeapProfileTranslator : ProfileTranslator { } v8::Local CreateAllocation(v8::Local count, - v8::Local sizeBytes) - GENERAL_REGS_ONLY { + v8::Local sizeBytes) { v8::Local js_alloc = NewObject(); #define X(name) Set(js_alloc, str_##name, name); ALLOCATION_FIELDS diff --git a/bindings/translate-time-profile.cc b/bindings/translate-time-profile.cc index 3772ebe1..41315648 100644 --- a/bindings/translate-time-profile.cc +++ b/bindings/translate-time-profile.cc @@ -15,6 +15,7 @@ */ #include "translate-time-profile.hh" +#include "general-regs-only.hh" #include "profile-translator.hh" namespace dd { @@ -40,8 +41,8 @@ class TimeProfileTranslator : ProfileTranslator { FIELDS #undef X - v8::Local getContextsForNode( - const v8::CpuProfileNode* node, uint32_t& hitcount) GENERAL_REGS_ONLY { + v8::Local getContextsForNode(const v8::CpuProfileNode* node, + uint32_t& hitcount) { hitcount = node->GetHitCount(); if (!contextsByNode) { // custom contexts are not enabled, keep the node hitcount and return @@ -70,8 +71,7 @@ class TimeProfileTranslator : ProfileTranslator { v8::Local columnNumber, v8::Local hitCount, v8::Local children, - v8::Local contexts) - GENERAL_REGS_ONLY { + v8::Local contexts) { v8::Local js_node = NewObject(); #define X(name) Set(js_node, str_##name, name); FIELDS @@ -133,8 +133,7 @@ class TimeProfileTranslator : ProfileTranslator { } v8::Local TranslateLineNumbersTimeProfileNode( - const v8::CpuProfileNode* parent, - const v8::CpuProfileNode* node) GENERAL_REGS_ONLY { + const v8::CpuProfileNode* parent, const v8::CpuProfileNode* node) { return CreateTimeNode(parent->GetFunctionName(), parent->GetScriptResourceName(), NewInteger(parent->GetScriptId()), @@ -149,7 +148,7 @@ class TimeProfileTranslator : ProfileTranslator { // and column number refer to the line/column from which the function was // called. v8::Local TranslateLineNumbersTimeProfileRoot( - const v8::CpuProfileNode* node) GENERAL_REGS_ONLY { + const v8::CpuProfileNode* node) { int32_t count = node->GetChildrenCount(); std::vector> childrenArrs(count); int32_t childCount = 0; @@ -180,8 +179,8 @@ class TimeProfileTranslator : ProfileTranslator { emptyArray); } - v8::Local TranslateTimeProfileNode(const v8::CpuProfileNode* node) - GENERAL_REGS_ONLY { + v8::Local TranslateTimeProfileNode( + const v8::CpuProfileNode* node) { int32_t count = node->GetChildrenCount(); v8::Local children = NewArray(count); for (int32_t i = 0; i < count; i++) { @@ -208,8 +207,7 @@ class TimeProfileTranslator : ProfileTranslator { v8::Local TranslateTimeProfile(const v8::CpuProfile* profile, bool includeLineInfo, bool hasCpuTime, - int64_t nonJSThreadsCpuTime) - GENERAL_REGS_ONLY { + int64_t nonJSThreadsCpuTime) { v8::Local js_profile = NewObject(); if (includeLineInfo) { diff --git a/bindings/translate-time-profile.hh b/bindings/translate-time-profile.hh index 94e46d86..6ba28c60 100644 --- a/bindings/translate-time-profile.hh +++ b/bindings/translate-time-profile.hh @@ -18,7 +18,6 @@ #include #include "contexts.hh" -#include "general-regs-only.hh" namespace dd { @@ -27,6 +26,6 @@ v8::Local TranslateTimeProfile( bool includeLineInfo, std::shared_ptr contextsByNode = nullptr, bool hasCpuTime = false, - int64_t nonJSThreadsCpuTime = 0) GENERAL_REGS_ONLY; + int64_t nonJSThreadsCpuTime = 0); } // namespace dd From 94cd54681b23ec6a5b6b1ab449bf59a2d8dbb464 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Mon, 31 Mar 2025 11:41:16 +0200 Subject: [PATCH 2/3] Revert "Prevent GCC using FP registers for pointer storage on arm64 Linux (#193)" This reverts commit 7b9cc3bba7e73929fdbee71eb5381db3ce1b8b8d. --- binding.gyp | 1 - bindings/general-regs-only.hh | 23 -------------------- bindings/profile-translator.cc | 22 ------------------- bindings/profile-translator.hh | 5 ++++- bindings/profilers/heap.hh | 3 +-- bindings/profilers/wall.cc | 35 ++++++++++++------------------ bindings/profilers/wall.hh | 19 +++++++--------- bindings/translate-heap-profile.hh | 3 +-- bindings/translate-time-profile.cc | 18 +++++++-------- bindings/translate-time-profile.hh | 2 +- 10 files changed, 37 insertions(+), 94 deletions(-) delete mode 100644 bindings/general-regs-only.hh delete mode 100644 bindings/profile-translator.cc diff --git a/binding.gyp b/binding.gyp index db93a543..d059d3c0 100644 --- a/binding.gyp +++ b/binding.gyp @@ -15,7 +15,6 @@ "bindings/profilers/heap.cc", "bindings/profilers/wall.cc", "bindings/per-isolate-data.cc", - "bindings/profile-translator.cc", "bindings/thread-cpu-clock.cc", "bindings/translate-heap-profile.cc", "bindings/translate-time-profile.cc", diff --git a/bindings/general-regs-only.hh b/bindings/general-regs-only.hh deleted file mode 100644 index cdcf8166..00000000 --- a/bindings/general-regs-only.hh +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2024 Datadog, Inc - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#if defined(__linux__) && defined(__aarch64__) -#define GENERAL_REGS_ONLY __attribute__((target("general-regs-only"))) -#else -#define GENERAL_REGS_ONLY -#endif diff --git a/bindings/profile-translator.cc b/bindings/profile-translator.cc deleted file mode 100644 index de2c18b4..00000000 --- a/bindings/profile-translator.cc +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright 2025 Datadog, Inc - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "profile-translator.hh" -#include - -v8::Local dd::ProfileTranslator::NewNumber(int64_t x) { - return v8::Number::New(isolate, static_cast(x)); -} diff --git a/bindings/profile-translator.hh b/bindings/profile-translator.hh index 54429c72..78ce070f 100644 --- a/bindings/profile-translator.hh +++ b/bindings/profile-translator.hh @@ -33,7 +33,10 @@ class ProfileTranslator { return v8::Boolean::New(isolate, x); } - v8::Local NewNumber(int64_t x); + template + v8::Local NewNumber(T x) { + return v8::Number::New(isolate, x); + } v8::Local NewArray(int length) { return length == 0 ? emptyArray : v8::Array::New(isolate, length); diff --git a/bindings/profilers/heap.hh b/bindings/profilers/heap.hh index 01c897e3..5badc46c 100644 --- a/bindings/profilers/heap.hh +++ b/bindings/profilers/heap.hh @@ -17,7 +17,6 @@ #pragma once #include -#include "general-regs-only.hh" namespace dd { @@ -35,7 +34,7 @@ class HeapProfiler { // getAllocationProfile(): AllocationProfileNode static NAN_METHOD(GetAllocationProfile); - static NAN_METHOD(MonitorOutOfMemory) GENERAL_REGS_ONLY; + static NAN_METHOD(MonitorOutOfMemory); static NAN_MODULE_INIT(Init); }; diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index fe805bfb..9a3dc0be 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -366,17 +366,10 @@ static int64_t GetV8ToEpochOffset() { return V8toEpochOffset; } -Local NewNumberFromInt64(Isolate* isolate, int64_t value) { - return Number::New(isolate, static_cast(value)); -} - -std::shared_ptr CreateContextsByNode() { - return std::make_shared(); -} - -std::shared_ptr WallProfiler::GetContextsByNode( - CpuProfile* profile, ContextBuffer& contexts, int64_t startCpuTime) { - auto contextsByNode = CreateContextsByNode(); +ContextsByNode WallProfiler::GetContextsByNode(CpuProfile* profile, + ContextBuffer& contexts, + int64_t startCpuTime) { + ContextsByNode contextsByNode; auto sampleCount = profile->GetSamplesCount(); if (contexts.empty() || sampleCount == 0) { @@ -436,11 +429,11 @@ std::shared_ptr WallProfiler::GetContextsByNode( break; } else { // This sample context is the closest to this sample. - auto it = contextsByNode->find(sample); + auto it = contextsByNode.find(sample); Local array; - if (it == contextsByNode->end()) { + if (it == contextsByNode.end()) { array = Array::New(isolate); - (*contextsByNode)[sample] = {array, 1}; + contextsByNode[sample] = {array, 1}; } else { array = it->second.contexts; ++it->second.hitcount; @@ -457,10 +450,10 @@ std::shared_ptr WallProfiler::GetContextsByNode( if (strcmp(function_name, "(program)") != 0) { if (collectCpuTime_) { timedContext - ->Set(v8Context, - cpuTimeKey, - NewNumberFromInt64(isolate, - sampleContext.cpu_time - lastCpuTime)) + ->Set( + v8Context, + cpuTimeKey, + Number::New(isolate, sampleContext.cpu_time - lastCpuTime)) .Check(); lastCpuTime = sampleContext.cpu_time; } @@ -478,7 +471,7 @@ std::shared_ptr WallProfiler::GetContextsByNode( timedContext ->Set(v8Context, asyncIdKey, - NewNumberFromInt64(isolate, sampleContext.async_id)) + Number::New(isolate, sampleContext.async_id)) .Check(); } } @@ -893,7 +886,7 @@ Result WallProfiler::StopImpl(bool restart, v8::Local& profile) { profile = TranslateTimeProfile(v8_profile, includeLines_, - contextsByNode, + &contextsByNode, collectCpuTime_, nonJSThreadsCpuTime); @@ -1069,7 +1062,7 @@ void WallProfiler::OnGCEnd() { void WallProfiler::PushContext(int64_t time_from, int64_t time_to, int64_t cpu_time, - int64_t async_id) { + double async_id) { // Be careful this is called in a signal handler context therefore all // operations must be async signal safe (in particular no allocations). // Our ring buffer avoids allocations. diff --git a/bindings/profilers/wall.hh b/bindings/profilers/wall.hh index 7b85c80d..a6b72534 100644 --- a/bindings/profilers/wall.hh +++ b/bindings/profilers/wall.hh @@ -17,7 +17,6 @@ #pragma once #include "contexts.hh" -#include "general-regs-only.hh" #include "thread-cpu-clock.hh" #include @@ -87,7 +86,7 @@ class WallProfiler : public Nan::ObjectWrap { int64_t time_from; int64_t time_to; int64_t cpu_time; - int64_t async_id; + double async_id; }; using ContextBuffer = std::vector; @@ -100,10 +99,9 @@ class WallProfiler : public Nan::ObjectWrap { // to work around https://bugs.chromium.org/p/v8/issues/detail?id=11051. v8::CpuProfiler* CreateV8CpuProfiler(); - std::shared_ptr GetContextsByNode(v8::CpuProfile* profile, - ContextBuffer& contexts, - int64_t startCpuTime) - GENERAL_REGS_ONLY; + ContextsByNode GetContextsByNode(v8::CpuProfile* profile, + ContextBuffer& contexts, + int64_t startCpuTime); bool waitForSignal(uint64_t targetCallCount = 0); @@ -129,11 +127,10 @@ class WallProfiler : public Nan::ObjectWrap { void PushContext(int64_t time_from, int64_t time_to, int64_t cpu_time, - int64_t async_id); + double async_id); Result StartImpl(); std::string StartInternal(); - Result StopImpl(bool restart, - v8::Local& profile) GENERAL_REGS_ONLY; + Result StopImpl(bool restart, v8::Local& profile); CollectionMode collectionMode() { auto res = collectionMode_.load(std::memory_order_relaxed); @@ -158,12 +155,12 @@ class WallProfiler : public Nan::ObjectWrap { void OnGCStart(v8::Isolate* isolate); void OnGCEnd(); - static NAN_METHOD(New) GENERAL_REGS_ONLY; + static NAN_METHOD(New); static NAN_METHOD(Start); static NAN_METHOD(Stop); static NAN_METHOD(V8ProfilerStuckEventLoopDetected); static NAN_METHOD(Dispose); - static NAN_MODULE_INIT(Init) GENERAL_REGS_ONLY; + static NAN_MODULE_INIT(Init); static NAN_GETTER(GetContext); static NAN_SETTER(SetContext); static NAN_GETTER(SharedArrayGetter); diff --git a/bindings/translate-heap-profile.hh b/bindings/translate-heap-profile.hh index 5493fba2..dc5c7aa6 100644 --- a/bindings/translate-heap-profile.hh +++ b/bindings/translate-heap-profile.hh @@ -17,11 +17,10 @@ #pragma once #include -#include "general-regs-only.hh" namespace dd { v8::Local TranslateAllocationProfile( - v8::AllocationProfile::Node* node) GENERAL_REGS_ONLY; + v8::AllocationProfile::Node* node); } // namespace dd diff --git a/bindings/translate-time-profile.cc b/bindings/translate-time-profile.cc index 41315648..6495d3f0 100644 --- a/bindings/translate-time-profile.cc +++ b/bindings/translate-time-profile.cc @@ -15,7 +15,6 @@ */ #include "translate-time-profile.hh" -#include "general-regs-only.hh" #include "profile-translator.hh" namespace dd { @@ -23,7 +22,7 @@ namespace dd { namespace { class TimeProfileTranslator : ProfileTranslator { private: - std::shared_ptr contextsByNode; + ContextsByNode* contextsByNode; v8::Local emptyArray = NewArray(0); v8::Local zero = NewInteger(0); @@ -81,7 +80,7 @@ class TimeProfileTranslator : ProfileTranslator { } v8::Local GetLineNumberTimeProfileChildren( - const v8::CpuProfileNode* node) GENERAL_REGS_ONLY { + const v8::CpuProfileNode* node) { unsigned int index = 0; v8::Local children; int32_t count = node->GetChildrenCount(); @@ -201,7 +200,7 @@ class TimeProfileTranslator : ProfileTranslator { } public: - explicit TimeProfileTranslator(std::shared_ptr nls = nullptr) + explicit TimeProfileTranslator(ContextsByNode* nls = nullptr) : contextsByNode(nls) {} v8::Local TranslateTimeProfile(const v8::CpuProfile* profile, @@ -231,12 +230,11 @@ class TimeProfileTranslator : ProfileTranslator { }; } // namespace -v8::Local TranslateTimeProfile( - const v8::CpuProfile* profile, - bool includeLineInfo, - std::shared_ptr contextsByNode, - bool hasCpuTime, - int64_t nonJSThreadsCpuTime) { +v8::Local TranslateTimeProfile(const v8::CpuProfile* profile, + bool includeLineInfo, + ContextsByNode* contextsByNode, + bool hasCpuTime, + int64_t nonJSThreadsCpuTime) { return TimeProfileTranslator(contextsByNode) .TranslateTimeProfile( profile, includeLineInfo, hasCpuTime, nonJSThreadsCpuTime); diff --git a/bindings/translate-time-profile.hh b/bindings/translate-time-profile.hh index 6ba28c60..c83115d5 100644 --- a/bindings/translate-time-profile.hh +++ b/bindings/translate-time-profile.hh @@ -24,7 +24,7 @@ namespace dd { v8::Local TranslateTimeProfile( const v8::CpuProfile* profile, bool includeLineInfo, - std::shared_ptr contextsByNode = nullptr, + ContextsByNode* contextsByNode = nullptr, bool hasCpuTime = false, int64_t nonJSThreadsCpuTime = 0); From d53acc8e1c43fbe4cf21828e8382f30f99fdefa9 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Mon, 31 Mar 2025 12:02:20 +0200 Subject: [PATCH 3/3] Can now keep asyncId as double --- bindings/profilers/wall.cc | 10 ++++------ bindings/profilers/wall.hh | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index 9a3dc0be..3769dbf0 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -1020,14 +1020,12 @@ NAN_METHOD(WallProfiler::Dispose) { delete profiler; } -int64_t GetAsyncIdNoGC(v8::Isolate* isolate) { - return isolate->InContext() - ? static_cast( - node::AsyncHooksGetExecutionAsyncId(isolate)) - : -1; +double GetAsyncIdNoGC(v8::Isolate* isolate) { + return isolate->InContext() ? node::AsyncHooksGetExecutionAsyncId(isolate) + : -1; } -int64_t WallProfiler::GetAsyncId(v8::Isolate* isolate) { +double WallProfiler::GetAsyncId(v8::Isolate* isolate) { if (!collectAsyncId_) { return -1; } diff --git a/bindings/profilers/wall.hh b/bindings/profilers/wall.hh index a6b72534..f7aac3b9 100644 --- a/bindings/profilers/wall.hh +++ b/bindings/profilers/wall.hh @@ -58,7 +58,7 @@ class WallProfiler : public Nan::ObjectWrap { std::atomic curContext_; std::atomic gcCount = 0; - int64_t gcAsyncId; + double gcAsyncId; std::atomic collectionMode_; std::atomic noCollectCallCount_; @@ -151,7 +151,7 @@ class WallProfiler : public Nan::ObjectWrap { return threadCpuStopWatch_.GetAndReset(); } - int64_t GetAsyncId(v8::Isolate* isolate); + double GetAsyncId(v8::Isolate* isolate); void OnGCStart(v8::Isolate* isolate); void OnGCEnd();