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

Ideas to speed up the popup? #58

Open
srsudar opened this issue May 3, 2021 · 10 comments
Open

Ideas to speed up the popup? #58

srsudar opened this issue May 3, 2021 · 10 comments

Comments

@srsudar
Copy link

srsudar commented May 3, 2021

Then I trigger the popup it opens instantaneously, but there is a noticeable lag before the tabs are populated. I'd say 500ms to 1s. The vimium tab switcher is completely instantaneous, though I like QuicKey's better. Vimium is also in the context of the page, rather than in a new context like the popup, so it is at an advantage.

Do you have any ideas about what might speed it up? I'm loading from dev rather than using the optimized version. Is React faster when running the optimized build?

I tried adding some additional timing information to the popup but I didn't see anything obvious. I was hoping that the chrome.connect function would be slow, or that loading the tabs would be slow. Doesn't look like it.

Maybe Require or React impose a minimum on startup?

Do you have any ideas about where the bottlenecks are?

@fwextensions
Copy link
Owner

Thanks for the report. A 1s render time for the initial popup is definitely too long. What hardware and OS are you running?

If you enable Developer mode in the top-right corner of the Extensions page in Chrome, you can inspect the console for QuicKey's background page. The dev build should show a startup time of ~250ms, while the built version should be ~50ms. (Enter DEBUG = true in the console to enable logging in the built version.) Either way, it should be faster than what you're seeing.

I'm working on another version of the extension that renders the menu in a persistent popup window, so it doesn't need to re-render every time it's shown, which makes it even faster. You can check it out on this branch.

@srsudar
Copy link
Author

srsudar commented May 4, 2021

It looks like I'm getting about 350ms in my dev version:

=== popup startup time 338.4900000237394 376.0000000183936

Fantastic about the optimized build, however. I'll start with that. I'm building from my own fork. I get the following error when I run grunt:

$ grunt
Running "build" task

Running "time" task
Started at 5/3/2021, 8:42:12 PM

Running "clean:rjs" (clean) task
>> 1 path cleaned.

Running "sync:out" (sync) task

Running "copy:quickey" (copy) task
Copied 1 file

Running "cleanupManifest:quickey:dev" (cleanupManifest) task

Running "buildPopup" task

Running "requirejs:modulesConfig" (requirejs) task

Running "babel:rjs" (babel) task
Warning: <snip>/QuicKey/build/rjs/common/base.js: Cannot read property 'add' of undefined Use --force to continue.

Is that the wrong way to build it?

The persistent popup window sounds great too.

@fwextensions
Copy link
Owner

Ah, looks like I haven't merged the fix for that from the dev branch onto master. Let me do that. Some combination of packages seems to trigger this bug in babel, so I replaced it with terser, which seems to be working fine.

(In general, apologies for the state of the repo. I started it when I was even more of a git newbie than I am now, so there's some wonkiness with the branches. I usually do all the work on dev, and then squash that onto master.)

@fwextensions
Copy link
Owner

Okay, master has been updated with the terser fix. Let me know if you still see issues.

@srsudar
Copy link
Author

srsudar commented May 5, 2021

Ah great! I merged master and after grunt it loads much faster. The dir I am loading is build/out, however, not build/prod (which is missing a manifest.json). It shows up now at chrome://extensions as QuicKey DEV BUILD Instead of just QuicKey DEV when I loaded straight from source.

Does that sound correct? I'd expected the build/prod directory at first.

Either way, much faster now!

I forgot to mention my hardware--I'm using a mbpro that is a couple years old. Not especially beefy but I don't think too underpowered either.

And no worries at all about git. The repo doesn't strike me as out of order, and we're all git newbies anyways. =) I don't even rebase, I just merge like a curmudgeon.

@fwextensions
Copy link
Owner

The paths are a little confusing. build/prod has prod versions of the HTML files that get copied to build/out, along with the JS files bundled by RequireJS. The built version's tooltip should also have a timestamp from when grunt was run. I usually have the Chrome store version, the raw source versions, and the built version all running, so having different names in the manifests helps distinguish them.

Are you seeing any difference in speed between the store version and your local built version? They should perform the same.

@srsudar
Copy link
Author

srsudar commented May 6, 2021

Ah, I see. The locally built version and the store version perform the same, as far as I can tell. I think I just needed the optimized build.

Is the React-free version of the UI also ready to play around with? I think I'm satisfied with the speed as it is now, but if you want more eyes on the other version I'm happy to try running it.

@fwextensions
Copy link
Owner

fwextensions commented May 7, 2021

The popup window version still uses React. It just shows the menu in a popup window and then keeps that around when it's hidden, either behind another window, in a tab or minimized (there are different tradeoffs). That way, the whole process doesn't have to restart when it's reshown, unlike the drop-down from the toolbar, so it's noticeably faster.

The other advantage of the popup is that it lets you navigate with a single key. So if ctrl-Q opens it, you can keep holding ctrl and press Q to navigate down the menu, then release ctrl to open the selected tab. With the drop-down version, you can't do that, as pressing the shortcut that opened it just closes it again, without any chance of interception. This lets the UI work more like the alt-tab popup, where you can keep pressing that shortcut to navigate the popup while holding alt.

It would be great if you could check it out, especially since I haven't done a ton of testing on Mac. It's on the feature/popup-window-ui branch. You'll need to reload QuicKey on the Extensions page, as this branch has two new keyboard shortcuts for opening it in a popup vs. the menu. It would be interesting to see if those make sense to you, as well as the popup window options on the Options page.

@srsudar
Copy link
Author

srsudar commented May 16, 2021

Finally got around to trying this. It works for me on Mac. The merge with my other branch with the extra config was pretty hairy for some reason, so I didn't leave it on for long.

It seemed to work though. It was almost instantaneous pulling to the front, which was great. I could see keeping it up as kind of a tab overview as well. The only thing I didn't love was that dismissing it was harder. In the current version if I search for something and can't find it (say because my history isn't going far enough back), I can use my custom escape shortcut to get rid of it. That's harder to do on this version.

Still seems like a nice option, though. The speed up especially I like quite a bit.

@fwextensions
Copy link
Owner

Thanks for checking it out. There are definitely a lot of changes on that branch, so merging may be tricky. It would probably be simpler to just reapply the additional esc shortcut changes on top of it.

By the way, I added ctrl-N/P and ctrl-J/K shortcuts to the 1.6.1 release, so I think the only thing missing is other shortcuts for closing the menu/popup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants