From 1b8d4e3adb47d76967dc95c525a249f9f6a9ea37 Mon Sep 17 00:00:00 2001 From: Camillo Bruni Date: Thu, 7 Apr 2022 14:48:04 +0200 Subject: [PATCH] [api] Remove FLAG_log_api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit API logging has not been used in a while and we have valid alternatives: - Runtime call stats - Profiling - Timer events Together they make --log-api superfluous and we can remove it and reduce the number of branches when calling into the V8 API. Change-Id: Ie10f70b61ebdb82166270e7630ebcf20a27c4902 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3574549 Reviewed-by: Marja Hölttä Auto-Submit: Camillo Bruni Reviewed-by: Jakob Linke Commit-Queue: Jakob Linke Cr-Commit-Position: refs/heads/main@{#79904} --- src/api/api-arguments-inl.h | 57 ++++-------- src/api/api-macros.h | 9 +- src/api/api.cc | 132 ++++++++++++++-------------- src/builtins/builtins-api.cc | 1 - src/debug/debug-interface.cc | 2 +- src/execution/isolate.cc | 2 - src/flags/flag-definitions.h | 1 - src/init/v8.cc | 1 - src/logging/log.cc | 40 --------- src/logging/log.h | 24 ----- test/cctest/test-log.cc | 3 - tools/system-analyzer/app-model.mjs | 13 +-- tools/system-analyzer/index.html | 6 +- tools/system-analyzer/index.mjs | 28 +----- tools/system-analyzer/log/api.mjs | 24 ----- tools/system-analyzer/processor.mjs | 20 +---- 16 files changed, 95 insertions(+), 268 deletions(-) delete mode 100644 tools/system-analyzer/log/api.mjs diff --git a/src/api/api-arguments-inl.h b/src/api/api-arguments-inl.h index 786f849be6..f832d7e501 100644 --- a/src/api/api-arguments-inl.h +++ b/src/api/api-arguments-inl.h @@ -87,24 +87,22 @@ inline JSReceiver FunctionCallbackArguments::holder() { ExternalCallbackScope call_scope(ISOLATE, FUNCTION_ADDR(F)); \ PropertyCallbackInfo callback_info(values_); -#define CREATE_NAMED_CALLBACK(FUNCTION, TYPE, RETURN_TYPE, API_RETURN_TYPE, \ - INFO_FOR_SIDE_EFFECT) \ - Handle PropertyCallbackArguments::CallNamed##FUNCTION( \ - Handle interceptor, Handle name) { \ - DCHECK_NAME_COMPATIBLE(interceptor, name); \ - Isolate* isolate = this->isolate(); \ - RCS_SCOPE(isolate, RuntimeCallCounterId::kNamed##FUNCTION##Callback); \ - Handle receiver_check_unsupported; \ - GenericNamedProperty##FUNCTION##Callback f = \ - ToCData( \ - interceptor->TYPE()); \ - PREPARE_CALLBACK_INFO(isolate, f, Handle, API_RETURN_TYPE, \ - INFO_FOR_SIDE_EFFECT, receiver_check_unsupported, \ - NotAccessor); \ - LOG(isolate, \ - ApiNamedPropertyAccess("interceptor-named-" #TYPE, holder(), *name)); \ - f(v8::Utils::ToLocal(name), callback_info); \ - return GetReturnValue(isolate); \ +#define CREATE_NAMED_CALLBACK(FUNCTION, TYPE, RETURN_TYPE, API_RETURN_TYPE, \ + INFO_FOR_SIDE_EFFECT) \ + Handle PropertyCallbackArguments::CallNamed##FUNCTION( \ + Handle interceptor, Handle name) { \ + DCHECK_NAME_COMPATIBLE(interceptor, name); \ + Isolate* isolate = this->isolate(); \ + RCS_SCOPE(isolate, RuntimeCallCounterId::kNamed##FUNCTION##Callback); \ + Handle receiver_check_unsupported; \ + GenericNamedProperty##FUNCTION##Callback f = \ + ToCData( \ + interceptor->TYPE()); \ + PREPARE_CALLBACK_INFO(isolate, f, Handle, API_RETURN_TYPE, \ + INFO_FOR_SIDE_EFFECT, receiver_check_unsupported, \ + NotAccessor); \ + f(v8::Utils::ToLocal(name), callback_info); \ + return GetReturnValue(isolate); \ } FOR_EACH_CALLBACK(CREATE_NAMED_CALLBACK) @@ -123,8 +121,6 @@ FOR_EACH_CALLBACK(CREATE_NAMED_CALLBACK) PREPARE_CALLBACK_INFO(isolate, f, Handle, API_RETURN_TYPE, \ INFO_FOR_SIDE_EFFECT, receiver_check_unsupported, \ NotAccessor); \ - LOG(isolate, ApiIndexedPropertyAccess("interceptor-indexed-" #TYPE, \ - holder(), index)); \ f(index, callback_info); \ return GetReturnValue(isolate); \ } @@ -136,7 +132,6 @@ FOR_EACH_CALLBACK(CREATE_INDEXED_CALLBACK) Handle FunctionCallbackArguments::Call(CallHandlerInfo handler) { Isolate* isolate = this->isolate(); - LOG(isolate, ApiObjectAccess("call", holder())); RCS_SCOPE(isolate, RuntimeCallCounterId::kFunctionCallback); v8::FunctionCallback f = v8::ToCData(handler.callback()); @@ -156,7 +151,6 @@ Handle FunctionCallbackArguments::Call(CallHandlerInfo handler) { Handle PropertyCallbackArguments::CallNamedEnumerator( Handle interceptor) { DCHECK(interceptor->is_named()); - LOG(isolate(), ApiObjectAccess("interceptor-named-enumerator", holder())); RCS_SCOPE(isolate(), RuntimeCallCounterId::kNamedEnumeratorCallback); return CallPropertyEnumerator(interceptor); } @@ -164,7 +158,6 @@ Handle PropertyCallbackArguments::CallNamedEnumerator( Handle PropertyCallbackArguments::CallIndexedEnumerator( Handle interceptor) { DCHECK(!interceptor->is_named()); - LOG(isolate(), ApiObjectAccess("interceptor-indexed-enumerator", holder())); RCS_SCOPE(isolate(), RuntimeCallCounterId::kIndexedEnumeratorCallback); return CallPropertyEnumerator(interceptor); } @@ -174,8 +167,6 @@ Handle PropertyCallbackArguments::CallNamedGetter( DCHECK_NAME_COMPATIBLE(interceptor, name); Isolate* isolate = this->isolate(); RCS_SCOPE(isolate, RuntimeCallCounterId::kNamedGetterCallback); - LOG(isolate, - ApiNamedPropertyAccess("interceptor-named-getter", holder(), *name)); GenericNamedPropertyGetterCallback f = ToCData(interceptor->getter()); return BasicCallNamedGetterCallback(f, name, interceptor); @@ -186,8 +177,6 @@ Handle PropertyCallbackArguments::CallNamedDescriptor( DCHECK_NAME_COMPATIBLE(interceptor, name); Isolate* isolate = this->isolate(); RCS_SCOPE(isolate, RuntimeCallCounterId::kNamedDescriptorCallback); - LOG(isolate, - ApiNamedPropertyAccess("interceptor-named-descriptor", holder(), *name)); GenericNamedPropertyDescriptorCallback f = ToCData( interceptor->descriptor()); @@ -215,8 +204,6 @@ Handle PropertyCallbackArguments::CallNamedSetter( RCS_SCOPE(isolate, RuntimeCallCounterId::kNamedSetterCallback); PREPARE_CALLBACK_INFO_FAIL_SIDE_EFFECT_CHECK(isolate, f, Handle, v8::Value); - LOG(isolate, - ApiNamedPropertyAccess("interceptor-named-set", holder(), *name)); f(v8::Utils::ToLocal(name), v8::Utils::ToLocal(value), callback_info); return GetReturnValue(isolate); } @@ -231,8 +218,6 @@ Handle PropertyCallbackArguments::CallNamedDefiner( ToCData(interceptor->definer()); PREPARE_CALLBACK_INFO_FAIL_SIDE_EFFECT_CHECK(isolate, f, Handle, v8::Value); - LOG(isolate, - ApiNamedPropertyAccess("interceptor-named-define", holder(), *name)); f(v8::Utils::ToLocal(name), desc, callback_info); return GetReturnValue(isolate); } @@ -246,8 +231,6 @@ Handle PropertyCallbackArguments::CallIndexedSetter( ToCData(interceptor->setter()); PREPARE_CALLBACK_INFO_FAIL_SIDE_EFFECT_CHECK(isolate, f, Handle, v8::Value); - LOG(isolate, - ApiIndexedPropertyAccess("interceptor-indexed-set", holder(), index)); f(index, v8::Utils::ToLocal(value), callback_info); return GetReturnValue(isolate); } @@ -262,8 +245,6 @@ Handle PropertyCallbackArguments::CallIndexedDefiner( ToCData(interceptor->definer()); PREPARE_CALLBACK_INFO_FAIL_SIDE_EFFECT_CHECK(isolate, f, Handle, v8::Value); - LOG(isolate, - ApiIndexedPropertyAccess("interceptor-indexed-define", holder(), index)); f(index, desc, callback_info); return GetReturnValue(isolate); } @@ -273,8 +254,6 @@ Handle PropertyCallbackArguments::CallIndexedGetter( DCHECK(!interceptor->is_named()); Isolate* isolate = this->isolate(); RCS_SCOPE(isolate, RuntimeCallCounterId::kNamedGetterCallback); - LOG(isolate, - ApiIndexedPropertyAccess("interceptor-indexed-getter", holder(), index)); IndexedPropertyGetterCallback f = ToCData(interceptor->getter()); return BasicCallIndexedGetterCallback(f, index, interceptor); @@ -285,8 +264,6 @@ Handle PropertyCallbackArguments::CallIndexedDescriptor( DCHECK(!interceptor->is_named()); Isolate* isolate = this->isolate(); RCS_SCOPE(isolate, RuntimeCallCounterId::kIndexedDescriptorCallback); - LOG(isolate, ApiIndexedPropertyAccess("interceptor-indexed-descriptor", - holder(), index)); IndexedPropertyDescriptorCallback f = ToCData(interceptor->descriptor()); return BasicCallIndexedGetterCallback(f, index, interceptor); @@ -323,7 +300,6 @@ Handle PropertyCallbackArguments::CallAccessorGetter( Handle info, Handle name) { Isolate* isolate = this->isolate(); RCS_SCOPE(isolate, RuntimeCallCounterId::kAccessorGetterCallback); - LOG(isolate, ApiNamedPropertyAccess("accessor-getter", holder(), *name)); AccessorNameGetterCallback f = ToCData(info->getter()); return BasicCallNamedGetterCallback(f, name, info, @@ -339,7 +315,6 @@ Handle PropertyCallbackArguments::CallAccessorSetter( ToCData(accessor_info->setter()); PREPARE_CALLBACK_INFO(isolate, f, Handle, void, accessor_info, handle(receiver(), isolate), Setter); - LOG(isolate, ApiNamedPropertyAccess("accessor-setter", holder(), *name)); f(v8::Utils::ToLocal(name), v8::Utils::ToLocal(value), callback_info); return GetReturnValue(isolate); } diff --git a/src/api/api-macros.h b/src/api/api-macros.h index 07b2e2d0f2..9fbe9a9739 100644 --- a/src/api/api-macros.h +++ b/src/api/api-macros.h @@ -35,10 +35,9 @@ * TODO(verwaest): Remove calls form API methods to DO_NOT_USE macros. */ -#define LOG_API(isolate, class_name, function_name) \ - RCS_SCOPE(isolate, \ - i::RuntimeCallCounterId::kAPI_##class_name##_##function_name); \ - LOG(isolate, ApiEntryCall("v8::" #class_name "::" #function_name)) +#define API_RCS_SCOPE(isolate, class_name, function_name) \ + RCS_SCOPE(isolate, \ + i::RuntimeCallCounterId::kAPI_##class_name##_##function_name); #define ENTER_V8_DO_NOT_USE(isolate) i::VMState __state__((isolate)) @@ -50,7 +49,7 @@ } \ HandleScopeClass handle_scope(isolate); \ CallDepthScope call_depth_scope(isolate, context); \ - LOG_API(isolate, class_name, function_name); \ + API_RCS_SCOPE(isolate, class_name, function_name); \ i::VMState __state__((isolate)); \ bool has_pending_exception = false diff --git a/src/api/api.cc b/src/api/api.cc index 1940281819..5ae0c0fa30 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -814,7 +814,7 @@ namespace internal { i::Address* GlobalizeTracedReference(i::Isolate* isolate, i::Address* obj, internal::Address* slot, GlobalHandleStoreMode store_mode) { - LOG_API(isolate, TracedGlobal, New); + API_RCS_SCOPE(isolate, TracedGlobal, New); #ifdef DEBUG Utils::ApiCheck((slot != nullptr), "v8::GlobalizeTracedReference", "the address slot must be not null"); @@ -847,7 +847,7 @@ void DisposeTracedReference(internal::Address* location) { namespace api_internal { i::Address* GlobalizeReference(i::Isolate* isolate, i::Address* obj) { - LOG_API(isolate, Persistent, New); + API_RCS_SCOPE(isolate, Persistent, New); i::Handle result = isolate->global_handles()->Create(*obj); #ifdef VERIFY_HEAP if (i::FLAG_verify_heap) { @@ -1338,7 +1338,7 @@ Local FunctionTemplate::New( i::Isolate* i_isolate = reinterpret_cast(isolate); // Changes to the environment cannot be captured in the snapshot. Expect no // function templates when the isolate is created for serialization. - LOG_API(i_isolate, FunctionTemplate, New); + API_RCS_SCOPE(i_isolate, FunctionTemplate, New); if (!Utils::ApiCheck( !c_function || behavior == ConstructorBehavior::kThrow, @@ -1363,7 +1363,7 @@ Local FunctionTemplate::NewWithCFunctionOverloads( SideEffectType side_effect_type, const MemorySpan& c_function_overloads) { i::Isolate* i_isolate = reinterpret_cast(isolate); - LOG_API(i_isolate, FunctionTemplate, New); + API_RCS_SCOPE(i_isolate, FunctionTemplate, New); if (!Utils::ApiCheck( c_function_overloads.size() == 0 || @@ -1384,7 +1384,7 @@ Local FunctionTemplate::NewWithCache( Local data, Local signature, int length, SideEffectType side_effect_type) { i::Isolate* i_isolate = reinterpret_cast(isolate); - LOG_API(i_isolate, FunctionTemplate, NewWithCache); + API_RCS_SCOPE(i_isolate, FunctionTemplate, NewWithCache); ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate); return FunctionTemplateNew(i_isolate, callback, data, signature, length, ConstructorBehavior::kAllow, false, cache_property, @@ -1567,7 +1567,7 @@ Local ObjectTemplate::New( static Local ObjectTemplateNew( i::Isolate* isolate, v8::Local constructor, bool do_not_cache) { - LOG_API(isolate, ObjectTemplate, New); + API_RCS_SCOPE(isolate, ObjectTemplate, New); ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); i::Handle struct_obj = isolate->factory()->NewStruct( i::OBJECT_TEMPLATE_INFO_TYPE, i::AllocationType::kOld); @@ -2032,7 +2032,7 @@ Local