Repository: electron/electron (language: cpp)

Issue:
[Bug]: Performance regression in 24.x
### Preflight Checklist

- [X] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success.

### Electron Version

24.0.0-alpha.6

### What operating system are you using?

Ubuntu

### Operating System Version

Linux alex-NUC11PAHi7 5.19.0-31-generic #32-Ubuntu SMP PREEMPT_DYNAMIC Fri Jan 20 15:20:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

### What arch are you using?

x64

### Last Known Working Electron version

23.1.1

### Expected Behavior

24 as fast as 23

### Actual Behavior

Slower

### Testcase Gist URL

https://gist.github.com/419cb8aed0be96a28afa3077be0db4ee

### Additional Information

I just did some entirely unrelated debugging in Fiddle and found a noticeable delay when using 24.

In the Fiddle I'm consistently getting ~70ms (50 to 80) for 23.1.1 and ~200ms (180 to 210) for _any_ 24 alpha (1 through 6).

For anything more complex it is way more noticeable, but the hello world Fiddle is enough as PoC.


Discussion / hints:
@Prinzhorn Sorry, just want to verify - I took a look at the gist, is the specific measurement here for time to start/time to load webContents? Just wanted to make sure we're looking at the right thing as we debug  👀  Thanks for reporting this!
I chose this event seemingly at random. What I noticed was that it took longer than expected to launch a Fiddle with 24.x and this was the first event I tried to make this measurable. So no, unfortunately I cannot narrow it down any further. It's just what I felt, I _think_ opening the window is as fast as usual but then the window is blank for longer than with 23.x. That's what I originally noticed (content showing slower than with 23)
Here's a slightly updated Fiddle. I won't go any further to avoid duplicate work. And frankly I don't know what other events could be used best to narrow this down further. But it appears it's actually the loading itself.

https://gist.github.com/7fb4af51631340ef4bb2f21033c977c0

```
v23
did-start-loading: 1.358ms
did-finish-load: 75.106ms

v24
did-start-loading: 1.489ms
did-finish-load: 204.044ms
```
I don't see this on macOS:

```
v22
did-start-loading: 2.505ms
did-finish-load: 254.832ms

v23
did-start-loading: 2.747ms
did-finish-load: 262.561ms

v24
did-start-loading: 2.707ms
did-finish-load: 244.948ms
```

So I guess this is Linux-specific..?
> So I guess this is Linux-specific..?

I could not reproduce in a Windows VM either.

Out of curiosity I added one more metric: https://gist.github.com/ac076574ef7e619cc7a1c304a4f9b031

```
v23
did-start-loading: 1.114ms
got-request: 15.619ms
did-finish-load: 62.397ms

v24
did-start-loading: 1.784ms
got-request: 68.516ms
did-finish-load: 180.419ms
```

Not sure this helps, because it makes it look like things are "just slow"? Because the requests takes longer to arrive but then it also takes longer to finish loading.
Can you also try measuring the time from app boot until did-finish-load? I wonder if some other initialization got moved around and you're now measuring that too, when you weren't previously.
Like this? It appears that v24 is slower in general, it's not just the slice I'm measuring (not WebContents related at all). Are there like debug symbols in the v24 Linux build or anything along those lines that make this slow in general?

```js
const { app, protocol, BrowserWindow } = require('electron')
const path = require('path')

function createWindow () {
  // Create the browser window.
  const mainWindow = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      preload: path.join(__dirname, 'preload.js')
    }
  })

  protocol.registerFileProtocol('atom', (request, callback) => {
    console.timeEnd('got-request');
    callback(path.join(__dirname, 'index.html'));
  })

  console.time('did-start-loading');
  console.time('did-finish-load');
  console.time('got-request');

  mainWindow.webContents.on('did-start-loading', () => {
    console.timeEnd('did-start-loading');
  });

  mainWindow.webContents.on('did-finish-load', () => {
    console.timeEnd('did-finish-load');
    app.quit();
  });

  // and load the index.html of the app.
  mainWindow.loadURL('atom://');
}

app.whenReady().then(() => {
  createWindow()
})
```

```
v23.1.4

 $ time ./node_modules/.bin/electron index.js 
did-start-loading: 1.352ms
got-request: 14.323ms
did-finish-load: 74.712ms

real	0m0,305s
user	0m0,220s
sys	0m0,039s

v24.0.0-beta.3

 $ time ./node_modules/.bin/electron index.js 
did-start-loading: 1.217ms
libva error: vaGetDriverNameByIndex() failed with unknown libva error, driver_name = (null)
got-request: 61.945ms
did-finish-load: 162.879ms

real	0m0,404s
user	0m0,251s
sys	0m0,163s
```

If I `quit()` in `did-start-loading` I get this:

```
v23.1.4

 $ time ./node_modules/.bin/electron index.js 
did-start-loading: 1.609ms
got-request: 15.61ms

real	0m0,232s
user	0m0,167s
sys	0m0,059s

v24.0.0-beta.3

did-start-loading: 1.37ms
libva error: vaGetDriverNameByIndex() failed with unknown libva error, driver_name = (null)
got-request: 76.883ms

real	0m0,358s
user	0m0,235s
sys	0m0,105s
```

It might be worth noting that v24 spits out `libva error: vaGetDriverNameByIndex() failed with unknown libva error, driver_name = (null)` which 23 does not.
If I look at the following it might be related to the window? I'll look into that.

```js
const { app } = require('electron')
app.quit();
```

```
v23.1.4

$ time ./node_modules/.bin/electron index.js 

real	0m0,230s
user	0m0,149s
sys	0m0,014s


v24.0.0-beta.3

$ time ./node_modules/.bin/electron index.js 

real	0m0,218s
user	0m0,145s
sys	0m0,027s
```

Edit: unfortunately I'm not well and can't look into this any further now.
`whenReady()` takes the same time in both. As does `new BrowserWindow()`. That means the time is lost after `loadURL` is called. So maybe it is an issue with WebContents after all. As seen from the original measurements it takes longer for the request to arrive in `registerFileProtocol` but then it also takes longer to `did-finish-load`. So both the networking part as well as the rendering are slower?
I wonder why this does not block stable? The time it takes for `did-finish-load` / `ready-to-show` has tripled. At least for me this will block upgrading to 24.
I have been experiencing an issue that might be similar, but not limited to v24 it seems to have originated back in v13

| version       | time till `ready-to-show` |
|---------------|--------------------------:|
| 11.5.0        |  208.279931ms             |
| 12.0.18       |  202.472953ms             |
| 12.1.2        |  203.189695ms             |
| 12.2.3        |  200.051848ms             |
| 13.0.1        | 1434.257709ms             |
| 13.6.9        | 1461.610284ms             |
| 14.0.2        | 1459.188428ms             |
| 23.2.0        | 1546.604452ms             |
| 24.0.0-beta.5 | 1601.363980ms             |

I have a intel integrated gpu and a discrete nvidia gpu that was recently disabled prompting me to start seeing this issue.

I have also checked the most recent version of chromium available to my machine  at this time `Chromium 111.0.5563.110` and it seems to load faster than electron.

Maybe this info will help you pin down when/where yours occurs.
I just did some triage on 37736, which as @Prinzhorn has pointed out is probably a duplicate of this ticket. 

If this is the same as 37736, it was introduced in https://github.com/electron/electron/compare/v24.0.0-nightly.20230202...v24.0.0-nightly.20230203 which happily only has two commits between them:

1. Chromium roll https://github.com/electron/electron/pull/36820. A roll would usually be the prime suspect; but here, the other commit is:
2. https://github.com/electron/electron/pull/37109 which sounds sus in the context of this bug report.

- FYI... I tried reverting https://github.com/electron/electron/pull/37109 on 24 and the issue still remains.
- Also, I tested against the latest nightly and the issue does not appear to be there.
FYI... some more context.  Looking at 25.0.0 nightlies, it looks like the issue was fixed in v25.0.0-nightly.20230322
Here's the diff to the previous nightly which has the problem: https://github.com/electron/electron/compare/v25.0.0-nightly.20230321...v25.0.0-nightly.20230322.

So it looks like this chromium roll fixed the issue:
- #37634


Fix patch:
diff --git a/DEPS b/DEPS
index 453f24f2fbbb0..d0eab1a543b30 100644
--- a/DEPS
+++ b/DEPS
@@ -2,7 +2,7 @@ gclient_gn_args_from = 'src'
 
 vars = {
   'chromium_version':
-    '112.0.5615.39',
+    '112.0.5615.49',
   'node_version':
     'v18.14.0',
   'nan_version':
diff --git a/patches/v8/.patches b/patches/v8/.patches
index d266dc584ca57..38d353921fd30 100644
--- a/patches/v8/.patches
+++ b/patches/v8/.patches
@@ -8,3 +8,4 @@ fix_build_deprecated_attribute_for_older_msvc_versions.patch
 fix_disable_implies_dcheck_for_node_stream_array_buffers.patch
 force_cppheapcreateparams_to_be_noncopyable.patch
 chore_allow_customizing_microtask_policy_per_context.patch
+store_the_thread_stack_start_in_tls.patch
diff --git a/patches/v8/store_the_thread_stack_start_in_tls.patch b/patches/v8/store_the_thread_stack_start_in_tls.patch
new file mode 100644
index 0000000000000..3a3ce98cc9ccf
--- /dev/null
+++ b/patches/v8/store_the_thread_stack_start_in_tls.patch
@@ -0,0 +1,193 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Victor Polevoy <fx@thefx.co>
+Date: Thu, 9 Mar 2023 12:06:55 +0100
+Subject: Store the thread stack start in TLS.
+
+Stores the thread's stack start address in the Thread Local Storage.
+This should allow for faster enter of isolates. Previously it was
+obtained each time whenever an isolate is entered what was costly
+in terms of performance and unnecessary in general. This change
+reverts the high performance back.
+
+Bug: v8:13772
+Change-Id: I0a5bb29579cfb407e7d129c4434192d2a6962296
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4338176
+Commit-Queue: Omer Katz <omerkatz@chromium.org>
+Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
+Reviewed-by: Omer Katz <omerkatz@chromium.org>
+Cr-Commit-Position: refs/heads/main@{#86578}
+(cherry picked from commit 2902d182818fa2790c2ff2b60979039fae4fc636)
+
+diff --git a/AUTHORS b/AUTHORS
+index 515bb0d6ccaf7e82b8feace2a49bbfcae29ecc8c..5f14dd472616aa17327fc3b44df4e13aa9dae078 100644
+--- a/AUTHORS
++++ b/AUTHORS
+@@ -258,6 +258,7 @@ Ujjwal Sharma <usharma1998@gmail.com>
+ Vadim Gorbachev <bmsdave@gmail.com>
+ Varun Varada <varuncvarada@gmail.com>
+ Victor Costan <costan@gmail.com>
++Victor Polevoy <fx@thefx.co>
+ Vlad Burlik <vladbph@gmail.com>
+ Vladimir Krivosheev <develar@gmail.com>
+ Vladimir Shutoff <vovan@shutoff.ru>
+diff --git a/BUILD.bazel b/BUILD.bazel
+index 08247f86115254220b954a0b7eddf22ff33002b1..99bd667de0007b87fd12ba3b6477e14e585dd2b9 100644
+--- a/BUILD.bazel
++++ b/BUILD.bazel
+@@ -663,6 +663,7 @@ filegroup(
+         "src/base/platform/mutex.cc",
+         "src/base/platform/mutex.h",
+         "src/base/platform/platform.h",
++        "src/base/platform/platform.cc",
+         "src/base/platform/semaphore.cc",
+         "src/base/platform/semaphore.h",
+         "src/base/platform/time.cc",
+diff --git a/BUILD.gn b/BUILD.gn
+index 1f8f89e1876ca7a02a8762c5e88d6fcf96037dc0..347471a52ce817618a3e9b7addc17e94b1e5619c 100644
+--- a/BUILD.gn
++++ b/BUILD.gn
+@@ -5648,6 +5648,7 @@ v8_component("v8_libbase") {
+     "src/base/platform/memory.h",
+     "src/base/platform/mutex.cc",
+     "src/base/platform/mutex.h",
++    "src/base/platform/platform.cc",
+     "src/base/platform/platform.h",
+     "src/base/platform/semaphore.cc",
+     "src/base/platform/semaphore.h",
+diff --git a/src/base/platform/platform-aix.cc b/src/base/platform/platform-aix.cc
+index 9c9adda3897b80ded70e8ddb3b6195eba5c98c47..e6421f37ac32a03e455c60e9ac58466e4447eca9 100644
+--- a/src/base/platform/platform-aix.cc
++++ b/src/base/platform/platform-aix.cc
+@@ -136,7 +136,7 @@ std::vector<OS::MemoryRange> OS::GetFreeMemoryRangesWithin(
+ }
+ 
+ // static
+-Stack::StackSlot Stack::GetStackStart() {
++Stack::StackSlot Stack::ObtainCurrentThreadStackStart() {
+   // pthread_getthrds_np creates 3 values:
+   // __pi_stackaddr, __pi_stacksize, __pi_stackend
+ 
+diff --git a/src/base/platform/platform-darwin.cc b/src/base/platform/platform-darwin.cc
+index b6260dfaf93e430f584c25eb37366fc94cad81e5..7fde32b22bf291b78bdc7e897995c662625794c4 100644
+--- a/src/base/platform/platform-darwin.cc
++++ b/src/base/platform/platform-darwin.cc
+@@ -136,7 +136,7 @@ std::vector<OS::MemoryRange> OS::GetFreeMemoryRangesWithin(
+ }
+ 
+ // static
+-Stack::StackSlot Stack::GetStackStart() {
++Stack::StackSlot Stack::ObtainCurrentThreadStackStart() {
+   return pthread_get_stackaddr_np(pthread_self());
+ }
+ 
+diff --git a/src/base/platform/platform-freebsd.cc b/src/base/platform/platform-freebsd.cc
+index 55d600283f6029ae28edf80eb68d405d079385a4..4314b1070928859e8a67b913ccfb334bb8737c2f 100644
+--- a/src/base/platform/platform-freebsd.cc
++++ b/src/base/platform/platform-freebsd.cc
+@@ -104,7 +104,7 @@ std::vector<OS::MemoryRange> OS::GetFreeMemoryRangesWithin(
+ }
+ 
+ // static
+-Stack::StackSlot Stack::GetStackStart() {
++Stack::StackSlot Stack::ObtainCurrentThreadStackStart() {
+   pthread_attr_t attr;
+   int error;
+   pthread_attr_init(&attr);
+diff --git a/src/base/platform/platform-posix.cc b/src/base/platform/platform-posix.cc
+index 540cc8bdff0e6c9a0365ea0e8cc27571472fe46d..40e870a346f885b9009d19f8567f28ad535432e3 100644
+--- a/src/base/platform/platform-posix.cc
++++ b/src/base/platform/platform-posix.cc
+@@ -1241,7 +1241,7 @@ void Thread::SetThreadLocal(LocalStorageKey key, void* value) {
+     !defined(V8_OS_SOLARIS)
+ 
+ // static
+-Stack::StackSlot Stack::GetStackStart() {
++Stack::StackSlot Stack::ObtainCurrentThreadStackStart() {
+   pthread_attr_t attr;
+   int error = pthread_getattr_np(pthread_self(), &attr);
+   if (!error) {
+diff --git a/src/base/platform/platform-solaris.cc b/src/base/platform/platform-solaris.cc
+index 8f5dd0c9f12284d5197695a9636e442f35fb4c93..6ff02e3278f9a486c36c4c6d3350c8e15f63ce74 100644
+--- a/src/base/platform/platform-solaris.cc
++++ b/src/base/platform/platform-solaris.cc
+@@ -72,7 +72,7 @@ std::vector<OS::MemoryRange> OS::GetFreeMemoryRangesWithin(
+ }
+ 
+ // static
+-Stack::StackSlot Stack::GetStackStart() {
++Stack::StackSlot Stack::ObtainCurrentThreadStackStart() {
+   pthread_attr_t attr;
+   int error;
+   pthread_attr_init(&attr);
+diff --git a/src/base/platform/platform-win32.cc b/src/base/platform/platform-win32.cc
+index 0eedaba0e512eea37fdbfdc109f82788851b5897..280fe18f7f2281884e7ced547762cda6feb98302 100644
+--- a/src/base/platform/platform-win32.cc
++++ b/src/base/platform/platform-win32.cc
+@@ -1739,7 +1739,7 @@ std::vector<OS::MemoryRange> OS::GetFreeMemoryRangesWithin(
+ }
+ 
+ // static
+-Stack::StackSlot Stack::GetStackStart() {
++Stack::StackSlot Stack::ObtainCurrentThreadStackStart() {
+ #if defined(V8_TARGET_ARCH_X64)
+   return reinterpret_cast<void*>(
+       reinterpret_cast<NT_TIB64*>(NtCurrentTeb())->StackBase);
+@@ -1753,7 +1753,7 @@ Stack::StackSlot Stack::GetStackStart() {
+   ::GetCurrentThreadStackLimits(&lowLimit, &highLimit);
+   return reinterpret_cast<void*>(highLimit);
+ #else
+-#error Unsupported GetStackStart.
++#error Unsupported ObtainCurrentThreadStackStart.
+ #endif
+ }
+ 
+diff --git a/src/base/platform/platform.cc b/src/base/platform/platform.cc
+new file mode 100644
+index 0000000000000000000000000000000000000000..3914b7dabab3fd32a85237cb1e7d97ad14f94d31
+--- /dev/null
++++ b/src/base/platform/platform.cc
+@@ -0,0 +1,29 @@
++// Copyright 2023 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/base/platform/platform.h"
++
++namespace v8 {
++
++namespace base {
++
++namespace {
++
++// A pointer to current thread's stack beginning.
++thread_local void* thread_stack_start = nullptr;
++
++}  // namespace
++
++// static
++Stack::StackSlot Stack::GetStackStart() {
++  DCHECK_IMPLIES(thread_stack_start,
++                 thread_stack_start == ObtainCurrentThreadStackStart());
++
++  if (!thread_stack_start) {
++    thread_stack_start = ObtainCurrentThreadStackStart();
++  }
++  return thread_stack_start;
++}
++
++}  // namespace base
++}  // namespace v8
+diff --git a/src/base/platform/platform.h b/src/base/platform/platform.h
+index b9b734dac8042b67bf68e8bace550c38130b5e7f..850f447185825cdce688e2687a2d0702904fb7e6 100644
+--- a/src/base/platform/platform.h
++++ b/src/base/platform/platform.h
+@@ -660,6 +660,10 @@ class V8_BASE_EXPORT Stack {
+ #endif  // V8_USE_ADDRESS_SANITIZER
+     return slot;
+   }
++
++ private:
++  // Returns the current thread stack start pointer.
++  static Stack::StackSlot ObtainCurrentThreadStackStart();
+ };
+ 
+ }  // namespace base


Test patch: