Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Limit maximum items in list or provide way to hide items. #80

Open
jordwalke opened this issue May 9, 2017 · 12 comments
Open

Limit maximum items in list or provide way to hide items. #80

jordwalke opened this issue May 9, 2017 · 12 comments

Comments

@jordwalke
Copy link

jordwalke commented May 9, 2017

There's an existing issue to allow omitting commands in the palette. I like the idea behind that feature request, but I want to make a more broad feature request: Make it so that the command palette isn't slow and sluggish when plugins add many commands. Issue #35 is just one potential solution so please close this issue and merge it with the other one if you think that #35 is the best approach. But there are other ways to fulfill my specific performance based feature request - you may truncate the list length at a configurable amount (ideally defaulted to some number like 50.

This is actually an important feature because one of the most popular (and imho important) packages for Atom (vim-mode-plus) necessarily implements many commands and these are all added to the command pallet. This makes the command pallet sluggish to open. I don't mind there being too many entries in the list from a searchability standpoint - typing filters effectively. I mind the slugishness when opening the command pallet and Atom feeling more bulky than it actually is due to VMP being such a beloved (and frequently installed) plugin.

Having this problem fixed by default out of the box when installing vmp is important because I'd love to be able to recommend Atom + VMP as a viable solution for long-time Vimmers, and know that the default out of box experience is going to feel great.

Another alternative solution is to incrementally render more rows, so that the initial opening is not blocked on rendering all of them. This is more complicated and probably doesn't improve the situation as effectively because it still renders many rows that are never seen by the user and creates a bunch of garbage for the system to manage that didn't provide much value. I doubt anyone opens the command palette and uses the down arrow to browser over 30, let alone 50 entries in the list - a compliment to the general user experience of the type-search experience.

wbinnssmith pushed a commit that referenced this issue Oct 24, 2017
This allows for commands to opt-into not appear in command palette.

Frequently commands are an implementation detail, or even only a means of
providing a keybinding, and do not benefit from being shown and selectable to
the user from the command palette.

Options like "Vim Mode Plus: Set Input Char M" can be confusing to the
user and cloud high-signal commands from surfacing in command palette
results.

Additionally, showing so many commands can [bog down the command palette and make it
slow to start](#80) --
though there are certainly other means of improving its performance.

The name is the most concise I could come up with to convey what we want
here. There might be other ways of surfacing commands in an interface
that might not want to obey this (or those that might), but for now I'm
erring on the side of being specific to command palette. Totally open to
making this more generic if there's a good argument or I'm missing
something.
wbinnssmith pushed a commit that referenced this issue Oct 24, 2017
This allows for commands to opt-into not appear in command palette.

Frequently commands are an implementation detail, or even only a means of
providing a keybinding, and do not benefit from being shown and selectable to
the user from the command palette.

Options like "Vim Mode Plus: Set Input Char M" can be confusing to the
user and cloud high-signal commands from surfacing in command palette
results.

Additionally, showing so many commands can [bog down the command palette and make it
slow to start](#80) --
though there are certainly other means of improving its performance.

The name is the most concise I could come up with to convey what we want
here. There might be other ways of surfacing commands in an interface
that might not want to obey this (or those that might), but for now I'm
erring on the side of being specific to command palette. Totally open to
making this more generic if there's a good argument or I'm missing
something.
wbinnssmith pushed a commit that referenced this issue Oct 24, 2017
This allows for commands to opt-into not appear in command palette.

Frequently commands are an implementation detail, or even only a means of
providing a keybinding, and do not benefit from being shown and selectable to
the user from the command palette.

Options like "Vim Mode Plus: Set Input Char M" can be confusing to the
user and cloud high-signal commands from surfacing in command palette
results.

Additionally, showing so many commands can [bog down the command palette and make it
slow to start](#80) --
though there are certainly other means of improving its performance.

The name is the most concise I could come up with to convey what we want
here. There might be other ways of surfacing commands in an interface
that might not want to obey this (or those that might), but for now I'm
erring on the side of being specific to command palette. Totally open to
making this more generic if there's a good argument or I'm missing
something.
wbinnssmith pushed a commit that referenced this issue Oct 24, 2017
This allows for commands to opt-into not appear in command palette.

Frequently commands are an implementation detail, or even only a means of
providing a keybinding, and do not benefit from being shown and selectable to
the user from the command palette.

Options like "Vim Mode Plus: Set Input Char M" can be confusing to the
user and cloud high-signal commands from surfacing in command palette
results.

Additionally, showing so many commands can [bog down the command palette and make it
slow to start](#80) --
though there are certainly other means of improving its performance.

The name is the most concise I could come up with to convey what we want
here. There might be other ways of surfacing commands in an interface
that might not want to obey this (or those that might), but for now I'm
erring on the side of being specific to command palette. Totally open to
making this more generic if there's a good argument or I'm missing
something.
wbinnssmith pushed a commit that referenced this issue Oct 24, 2017
This allows for commands to opt-into not appear in command palette.

Frequently commands are an implementation detail, or even only a means of
providing a keybinding, and do not benefit from being shown and selectable to
the user from the command palette.

Options like "Vim Mode Plus: Set Input Char M" can be confusing to the
user and cloud high-signal commands from surfacing in command palette
results.

Additionally, showing so many commands can [bog down the command palette and make it
slow to start](#80) --
though there are certainly other means of improving its performance.

The name is the most concise I could come up with to convey what we want
here. There might be other ways of surfacing commands in an interface
that might not want to obey this (or those that might), but for now I'm
erring on the side of being specific to command palette. Totally open to
making this more generic if there's a good argument or I'm missing
something.
wbinnssmith pushed a commit that referenced this issue Oct 24, 2017
This allows for commands to opt-into not appear in command palette.

Frequently commands are an implementation detail, or even only a means of
providing a keybinding, and do not benefit from being shown and selectable to
the user from the command palette.

Options like "Vim Mode Plus: Set Input Char M" can be confusing to the
user and cloud high-signal commands from surfacing in command palette
results.

Additionally, showing so many commands can [bog down the command palette and make it
slow to start](#80) --
though there are certainly other means of improving its performance.

The name is the most concise I could come up with to convey what we want
here. There might be other ways of surfacing commands in an interface
that might not want to obey this (or those that might), but for now I'm
erring on the side of being specific to command palette. Totally open to
making this more generic if there's a good argument or I'm missing
something.
wbinnssmith pushed a commit that referenced this issue Oct 24, 2017
This allows for commands to opt-into not appear in command palette.

Frequently commands are an implementation detail, or even only a means of
providing a keybinding, and do not benefit from being shown and selectable to
the user from the command palette.

Options like "Vim Mode Plus: Set Input Char M" can be confusing to the
user and cloud high-signal commands from surfacing in command palette
results.

Additionally, showing so many commands can [bog down the command palette and make it
slow to start](#80) --
though there are certainly other means of improving its performance.

The name is the most concise I could come up with to convey what we want
here. There might be other ways of surfacing commands in an interface
that might not want to obey this (or those that might), but for now I'm
erring on the side of being specific to command palette. Totally open to
making this more generic if there's a good argument or I'm missing
something.
@nathansobo
Copy link
Contributor

nathansobo commented Nov 16, 2017

Now that we allow commands to be hidden in #92 and are caching element items in #94, can this be closed? Or is the palette still sluggish to open when there are lots of commands? @jarle do you have thoughts on this?

@jarle
Copy link
Contributor

jarle commented Nov 16, 2017

Recent PRs have at least narrowed the problem down to the initial command-palette toggle. @t9md timed this to ~100ms(with several community packages installed) in #81, which I think is acceptable. Based on that, I think it is fair to close this issue. If opening the palette continues to feel sluggish for some after these changes, it might be interesting to collect some numbers on how long the toggle actually takes on their hardware.

@t9md
Copy link
Contributor

t9md commented Nov 16, 2017

I checked, latest master(with caching feature enabled).

In short, although it reduce some perf issue, I still feel lag.
What I'm focusing(I believe it sync with @jordwalke 's intention) is initial-open response time.

Work flow i'm imagining is

  • Open atom window and immediately hit cmd-shift-p to invoke some command.
  • After a while open another new atom window and hit cmd-shift-p immediately and so on.

Initial palette open lag is most important and caching is nothing to do with that(as far as I understand).
Still limiting maxResults is really effective, it reduce items to render(reduce num of element attached to DOM).

As experiment I reproduced quick time-measure I did in #81 with latest master.
Here is how I added code t9md#1.

  • So in my local env with vim-mode-plus pkg + other pkg, total num of command reaches 738.
  • Total time spent on 1st command-palette:toggle is 263ms(no limit) vs 93ms(max 20)
  • This is big diff. When total ms exceeds 100ms, I can feel lag(I believe everyone can).

untitled_2

I will continued experiment to find apply maxResult without confusing user to reduce initial open response lag.

@jarle
Copy link
Contributor

jarle commented Nov 16, 2017

Thanks for benchmarking! I have implemented a solution in #97 where items are added incrementally. This change should significantly speed up launching the command palette.

@jarle
Copy link
Contributor

jarle commented Nov 16, 2017

Benchmarks with 810 items:

image

"Initial items" is when the palette is initially visible, and "All items" is when all items has been rendered. Initial render is 78ms when not cached, and 30-40ms when cached.

@t9md t9md mentioned this issue Nov 16, 2017
@t9md
Copy link
Contributor

t9md commented Nov 16, 2017

Hoo, fantastic!!
Just after I send #98 to show different idea. Maybe this was unnecessary!? 😅

@t9md
Copy link
Contributor

t9md commented Nov 16, 2017

@jarle Number of command info is necessary info to evaluate result of bench.
My case is total 738 commands .
Can you show bench with same console.log insertion on view.show method?
Want to see total, render, collect, same output.

@jarle
Copy link
Contributor

jarle commented Nov 16, 2017

More solutions to a problem can never be a bad thing 🙂
I used a total of 810 items for the benchmark.

@jarle
Copy link
Contributor

jarle commented Nov 16, 2017

image

@jordwalke
Copy link
Author

Can't wait for this to be addressed - regardless of which solution is used. Yes, initial open latency has a huge impact on the perceived stability/performance of Atom in general, so I'm very happy to see progress here. Typing/update latency of the command palette also matters too of course.

@nathansobo
Copy link
Contributor

So which of the references PRs achieves the best perf?

@nathansobo
Copy link
Contributor

Never mind, looks like #98

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

No branches or pull requests

4 participants