From 199f6d64db9e52fc877a7db4bee069e318921edd Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 25 Feb 2025 19:20:33 -0600 Subject: [PATCH] perf: avoid redundant method calls in EventEmitter (#45786) * refactor: move EventEmitter::EmitWithEvent() into EventEmitter::Emit() * perf: remove redundant calls to isolate() in EventEmitter::Emit() * perf: remove redundant calls to GetWrapper() in EventEmitter::EmitEvent() * perf: remove redundant calls to isolate() in EventEmitter::EmitWithoutEvent() * perf: remove redundant calls to GetWrapper() in EventEmitter::EmitWithoutEvent() * refactor: remove unused method EventEmitter::isolate() * refactor: remove unused method EventEmitter::GetWrapper(v8::Isolate*) * refactor: remove unused method EventEmitter::GetWrapper() refactor: make the EventEmitter::Base typedef private * refactor: remove unused typedef EventEmitter::Base See "Workarounds" section in https://isocpp.org/wiki/faq/templates#nondependent-name-lookup-members * refactor: remove redundant gin_helper:: namespace use --- shell/common/gin_helper/event_emitter.h | 45 +++++++------------------ 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/shell/common/gin_helper/event_emitter.h b/shell/common/gin_helper/event_emitter.h index 74b8c7f379a5..2a9485696dd4 100644 --- a/shell/common/gin_helper/event_emitter.h +++ b/shell/common/gin_helper/event_emitter.h @@ -23,37 +23,30 @@ namespace gin_helper { template class EventEmitter : public gin_helper::Wrappable { public: - using Base = gin_helper::Wrappable; - - // Make the convenient methods visible: - // https://isocpp.org/wiki/faq/templates#nondependent-name-lookup-members - v8::Isolate* isolate() const { return Base::isolate(); } - v8::Local GetWrapper() const { return Base::GetWrapper(); } - v8::MaybeLocal GetWrapper(v8::Isolate* isolate) const { - return Base::GetWrapper(isolate); - } - // this.emit(name, new Event(), args...); template bool Emit(const std::string_view name, Args&&... args) { - v8::HandleScope handle_scope(isolate()); - v8::Local wrapper = GetWrapper(); + v8::Isolate* const isolate = this->isolate(); + v8::HandleScope handle_scope{isolate}; + v8::Local wrapper = this->GetWrapper(); if (wrapper.IsEmpty()) return false; - gin::Handle event = - internal::Event::New(isolate()); - return EmitWithEvent(name, event, std::forward(args)...); + gin::Handle event = internal::Event::New(isolate); + // It's possible that |this| will be deleted by EmitEvent, so save anything + // we need from |this| before calling EmitEvent. + EmitEvent(isolate, wrapper, name, event, std::forward(args)...); + return event->GetDefaultPrevented(); } // this.emit(name, args...); template void EmitWithoutEvent(const std::string_view name, Args&&... args) { - v8::HandleScope handle_scope(isolate()); - v8::Local wrapper = GetWrapper(); + v8::Isolate* const isolate = this->isolate(); + v8::HandleScope handle_scope{isolate}; + v8::Local wrapper = this->GetWrapper(); if (wrapper.IsEmpty()) return; - gin_helper::EmitEvent(isolate(), GetWrapper(), name, - std::forward(args)...); + EmitEvent(isolate, wrapper, name, std::forward(args)...); } // disable copy @@ -62,20 +55,6 @@ class EventEmitter : public gin_helper::Wrappable { protected: EventEmitter() = default; - - private: - // this.emit(name, event, args...); - template - bool EmitWithEvent(const std::string_view name, - gin::Handle event, - Args&&... args) { - // It's possible that |this| will be deleted by EmitEvent, so save anything - // we need from |this| before calling EmitEvent. - auto* isolate = this->isolate(); - gin_helper::EmitEvent(isolate, GetWrapper(), name, event, - std::forward(args)...); - return event->GetDefaultPrevented(); - } }; } // namespace gin_helper