Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Issue with Autofill Functionality in Vite + Vue 3 Application #12031

Open
1 task done
manuelgoi opened this issue Nov 18, 2024 · 12 comments · May be fixed by #12695
Open
1 task done

Performance Issue with Autofill Functionality in Vite + Vue 3 Application #12031

manuelgoi opened this issue Nov 18, 2024 · 12 comments · May be fixed by #12695
Assignees
Labels
browser Browser Extension bug

Comments

@manuelgoi
Copy link

manuelgoi commented Nov 18, 2024

Steps To Reproduce

  1. Start the application with Bitwarden extension enabled.
  2. Perform drag-and-drop operations on elements within any scrollable area.
  3. Observe the performance impact and frequent calls to requestIdleCallbackPolyfill() in Chrome's performance profile.

Expected Result

The autofill functionality should not significantly impact application performance and should handle idle callbacks more efficiently.

Actual Result

The autofill process significantly affects the application's responsiveness and performance, especially during drag-and-drop interactions within scrollable areas.

Screenshots or Videos

image

Additional Context

I am experiencing a significant performance issue in a web application developed using Vite, Vue 3, TypeScript, and Pinia. The issue becomes evident when performing drag-and-drop operations on elements within a layer that has overflow scrolling enabled. The problem is associated with the bootstrap-legacy-autofill-overlay.js file, specifically the requestIdleCallbackPolyfill() function.

Issue Details:

Environment: Vite + Vue 3 + TypeScript + Pinia
The performance profiling tools in Chrome DevTools indicate that the requestIdleCallbackPolyfill() function is frequently triggered and consumes a significant amount of CPU time. This function is called as part of the autofill process initiated by Bitwarden, and it severely impacts the application's performance, particularly during user interactions involving scrolling and element manipulation.

The profiling data shows repeated activations of the polyfill function, particularly when inactive scroll activation tasks are triggered. It appears that the autofill functionality is interacting in a way that significantly degrades performance during these operations.

Operating System

macOS, Linux

Operating System Version

Macos 15.1

Web Browser

Chrome

Browser Version

130.0.6723.117

Build Version

(Build oficial) (arm64)

Issue Tracking Info

  • I understand that work is tracked outside of Github. A PR will be linked to this issue should one be opened to address it, but Bitwarden doesn't use fields like "assigned", "milestone", or "project" to track progress.
@manuelgoi manuelgoi added browser Browser Extension bug labels Nov 18, 2024
@bitwarden-bot
Copy link

Thank you for reporting this issue! We've added this to our internal tracking system.
ID: PM-14992

@ejallier
Copy link

Hello, same problem for me.
My website is really slower when the extension is activated.
I have the error related to bootstrap-legacy-autofill-overlay.js

@cagonzalezcs
Copy link
Contributor

cagonzalezcs commented Nov 18, 2024

@jprusik

The performance issues described here are related to similar other reports in the past, for instance: #11077

Some recent improvements in the extension have helped with this kind of concern, but improving the performance cost of autofill is tricky. The main issue at hand is going to be the MutationObsevers that need to identify when changes occur in the DOM to re-capture form field data...

@manuelgoi
Copy link
Author

Hello,

First of all, I’d like to thank you for taking the time to address my report and for the continuous improvements you’ve made to Bitwarden. I greatly appreciate all the work that goes into maintaining such an essential tool.

After reviewing your response regarding the performance issue and understanding that the autofill functionality relies on MutationObservers, I still have one main question that I’d like to ask in a humble way.

Why is the requestIdleCallbackPolyfill() being used in the latest versions of Chrome, which natively supports requestIdleCallback? My understanding is that polyfills are generally meant to be active only when their native counterpart isn’t available or supported in the current environment. Could you help clarify why the polyfill is being triggered even in modern browsers where requestIdleCallback is fully implemented?

If it’s due to specific browser inconsistencies or additional functionality provided by the polyfill, I’d love to understand more about this decision, as it seems to impact performance in my use case (drag-and-drop interactions in scrollable areas).

I hope my question doesn’t come across as critical—I’m just trying to better understand how the extension operates and how I might mitigate the performance issue on my end.

Thank you again for your time and support!

@cagonzalezcs
Copy link
Contributor

cagonzalezcs commented Nov 19, 2024

@manuelgoi

requestCallbackPolyfill does call requiredIdleCallback in browsers that support it.

export function requestIdleCallbackPolyfill(

The method is a wrapper that falls back to setTimeout in cases where the API does not exist, such as in Safari.

Ultimately, the issue at hand likely comes down to the autofill feature having to act on mutations within the DOM, not the idle callback method itself.

If you have any public websites where this behavior can be observed, it'd likely help the Bitwarden developers resolve the issue... it's tricky to resolve a problem without a place where it can be tested.

@Zaczero
Copy link

Zaczero commented Dec 30, 2024

@cagonzalezcs I am noticing the same behavior and I have additional information.

I am using a very recent chromium browser and while requiredIdleCallback is there, it calls debounce method which uses timer, removing benefits of requiredIdleCallback and forcing renders during web use (non-idle).

I am attaching chrome profile from https://rapideditor.org where the issue is clearly present (open the editor and drag something around the map):
Trace-20241230T095016.json.gz

You can see see the mutations are fired by a timer:
image

...which is installed during requiredIdleCallback (and not executed):
image

This configuration causes the heavy computation to be done in a timer callback, while idle callback only installs the timer. Affecting the responsiveness and performance of web apps.

@Zaczero
Copy link

Zaczero commented Dec 30, 2024

PS. On the other side, processMutations seems to require further optimizations. Around 100ms to finish on a well specd device. It is called quite frequently too:

image

Perhaps by fine tuning observer options you would be able to skip processing lots of unrelated data, there's quite a lot flexibility here:
https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver/observe
(I haven't seen the code, I'm just guessing)

Edit: It could also help if some caching was added to skip rechecking the same elements for the shadow root existence. Basically, something needs to be done. Someone should open the JS profiler and see what's up.

@cagonzalezcs
Copy link
Contributor

cagonzalezcs commented Jan 3, 2025

@Zaczero

while requiredIdleCallback is there, it calls debounce method which uses timer, removing benefits of requiredIdleCallback and forcing renders during web use

Hmm... that's not entirely accurate.

The debounce was placed there to account for multiple mutations that occur in quick succession, ensuring that the list of mutations is processed at most every "x milliseconds".

Beyond that timeout, each mutation is then also processed within a subsequent requestIdleCallback call that is not debounced.

requestIdleCallbackPolyfill(processMutationRecords, { timeout: 500 });

The issue with this "jank" being introduced comes down to having to be thorough in checking for ShadowDOM elements within a set of mutations. It's pretty taxing given the number of tasks that need to be done when re-assessing if a form element has been added to the page (checking for visibility of the element, position, gather attributes, etc...). That's why nested calls to requestIdleCallback are used to attempt to avoid processing mutations until the event loop is free to do so without affecting user experience. While some improvements could still be made to that, it is definitely much more performant at this point than was previously when the ShadowDOM fix was first introduced.

I'm no longer working at Bitwarden, but based on recent PRs put up the by the team, I believe some work is being done to guard against injection of these kinds of content scripts on a per domain basis. That should help with the performance concerns that are found on certain SPA implementations.

Beyond that, I'm positive the team at Bitwarden is aware of your concern and it's likely that any concrete suggestions or ideas for how to improve this would be appreciated.

@jprusik jprusik self-assigned this Jan 3, 2025
@jprusik
Copy link
Contributor

jprusik commented Jan 3, 2025

Just to add; the Autofill team is mindful of this and performance improvements we can make, but we have nothing specific we can report on just yet.

@Zaczero
Copy link

Zaczero commented Jan 3, 2025

@cagonzalezcs The screenshots from the profiler I have attached illustrate that the heavy computation is done during timer event and not idle callback. Near the top of the callstack there is a label indicating what triggered the function (it's visible on the screenshots).

But I also think this subtle bug only slightly affects the responsiveness. Even if you run 100ms method in an idle callback, you will get poor experience.

@cagonzalezcs
Copy link
Contributor

cagonzalezcs commented Jan 3, 2025

Then maybe what needs to happen is simply a debounce that isn't wrapped by the idle callback within the top level callback of the Mutation Observer.

The debounce only acts as a means to build a list of mutated nodes and act on those nodes once within a 100ms period, rather than at the time they are inserted into the DOM.

Basically flip the two calls around, have the debounce eventually call the idle callback method.

cagonzalezcs added a commit to cagonzalezcs/bitwarden-clients that referenced this issue Jan 5, 2025
…s opposed to call requestIdleCallback on debounce method

Potential fix for bitwarden#12031
@cagonzalezcs
Copy link
Contributor

cagonzalezcs commented Jan 5, 2025

@jprusik

This might resolve the issue that @Zaczero is speaking about...

It's worth checking to ensure that Shadow DOM elements are still being filled as expected... not sure if tests pass on this, doubt it.

main...cagonzalezcs:bitwarden-clients:patch-1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser Browser Extension bug
Projects
None yet
6 participants