Repository: electron/electron
Pull request: perf: have `ErrorThrower` lazily lookup the current isolate

#### Description of Change

ErrorThrower's default constructor is commented this way:

```
// This constructor should be rarely if ever used, since
// v8::Isolate::GetCurrent() uses atomic loads and is thus a bit
// costly to invoke
```

Unfortunately, nearly every instance of ErrorThrower comes as an argument in gin_helper's JavaScript-to-C++ marshalling code where a [thrower is default-constructed](https://github.com/electron/electron/blob/09135443a04c2155198adc29515c1f9b247ff52e/shell/common/gin_helper/constructor.h#L33) and then [populated in gin_helper::GetNextArgument()](https://github.com/electron/electron/blob/09135443a04c2155198adc29515c1f9b247ff52e/shell/common/gin_helper/function_template.h#L152) with an assignment operator to a temporary `ErrorThrower` constructed with the `gin::Arguments`' isolate.

tldr: most of the time we use the slow constructor first, then throw that work away unused by overwriting with a fast-constructed one.

This refactor avoids that cost by deferring the expensive work until `ErrorThrower::isolate()` is called. Even then, it's only used as a fallback iff `isolate_` hasn't been set.

#### Checklist

- [x] PR description included and stakeholders cc'd
- [x] `npm test` passes
- [x] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: none.

Changed files:
filename: shell/common/gin_helper/error_thrower.cc
patch: @@ -10,12 +10,11 @@
 
 namespace gin_helper {
 
-ErrorThrower::ErrorThrower(v8::Isolate* isolate) : isolate_(isolate) {}
-
-// This constructor should be rarely if ever used, since
-// v8::Isolate::GetCurrent() uses atomic loads and is thus a bit
-// costly to invoke
-ErrorThrower::ErrorThrower() : isolate_(v8::Isolate::GetCurrent()) {}
+v8::Isolate* ErrorThrower::isolate() const {
+  // Callers should prefer to specify the isolate in the constructor,
+  // since GetCurrent() uses atomic loads and is thus a bit costly to invoke
+  return isolate_ ? isolate_.get() : v8::Isolate::GetCurrent();
+}
 
 void ErrorThrower::ThrowError(const std::string_view err_msg) const {
   Throw(v8::Exception::Error, err_msg);
@@ -39,9 +38,10 @@ void ErrorThrower::ThrowSyntaxError(const std::string_view err_msg) const {
 
 void ErrorThrower::Throw(ErrorGenerator gen,
                          const std::string_view err_msg) const {
-  v8::Local<v8::Value> exception = gen(gin::StringToV8(isolate_, err_msg), {});
-  if (!isolate_->IsExecutionTerminating())
-    isolate_->ThrowException(exception);
+  v8::Isolate* isolate = this->isolate();
+
+  if (!isolate->IsExecutionTerminating())
+    isolate->ThrowException(gen(gin::StringToV8(isolate, err_msg), {}));
 }
 
 }  // namespace gin_helper

filename: shell/common/gin_helper/error_thrower.h
patch: @@ -14,8 +14,8 @@ namespace gin_helper {
 
 class ErrorThrower {
  public:
-  explicit ErrorThrower(v8::Isolate* isolate);
-  ErrorThrower();
+  constexpr explicit ErrorThrower(v8::Isolate* isolate) : isolate_{isolate} {}
+  constexpr ErrorThrower() = default;
   ~ErrorThrower() = default;
 
   void ThrowError(std::string_view err_msg) const;
@@ -24,14 +24,14 @@ class ErrorThrower {
   void ThrowReferenceError(std::string_view err_msg) const;
   void ThrowSyntaxError(std::string_view err_msg) const;
 
-  v8::Isolate* isolate() const { return isolate_; }
+  v8::Isolate* isolate() const;
 
  private:
   using ErrorGenerator = v8::Local<v8::Value> (*)(v8::Local<v8::String> err_msg,
                                                   v8::Local<v8::Value> options);
   void Throw(ErrorGenerator gen, std::string_view err_msg) const;
 
-  raw_ptr<v8::Isolate> isolate_;
+  raw_ptr<v8::Isolate> isolate_ = {};
 };
 
 }  // namespace gin_helper