WIP: Boost performance #1673

kaste · 2019-10-04T21:23:38Z

Various performance commits.

A 
0 errors across 0 files from annotations. 150 errors in the store
group_by_filename_and_update took 35ms [D]  --> 4ms

0 errors across 0 files from flake8. 150 errors in the store
group_by_filename_and_update took 75ms [D]  --> 23ms

8 errors across 5 files from mypy. 150 errors in the store
group_by_filename_and_update took 113ms [D] --> 16ms

B
0 errors across 0 files from annotations. 150 errors in the store
group_by_filename_and_update took 166ms [D] --> 4ms

0 errors across 0 files from flake8. 150 errors in the store
group_by_filename_and_update took 127ms [D] --> 25ms

58 errors across 5 files from mypy. 150 errors in the store
group_by_filename_and_update took 292ms [D] --> 23ms

C
0 errors across 0 files from annotations. 150 errors in the store
group_by_filename_and_update took 83ms [D]  --> 8ms

0 errors across 0 files from flake8. 150 errors in the store
group_by_filename_and_update took 46ms [D]  --> 7ms

70 errors across 29 files from mypy. 150 errors in the store
group_by_filename_and_update took 975ms [D] --> 42ms

Must be read commit by commit.

Solves #1623 for free in a pragmatic way: if the results change it will open the panel again. Otherwise the panel stays closed.

settings are now generally cached in memory. Greatly improves settings.has_changed which was very cheap and idiosyncratic and now just works as anyone would expect.

Introduces new event lint_result_changed which otherwise is the same as our known 'lint_result' with a cache in front. Notable: Since we usually rely on our settings as a global, t.i. we're never pure anywhere probably, we use a settings 'change-count' as additional cache salt.

Fixes #1623
Fixes #1662
Fixes #1671

kaste · 2019-10-05T12:55:00Z

I don't like the complicated throttler in panel_view so I reverted e803a44 and the previous throttler 2cd32b7.

Now, 5284df0 and the new tip b76c6b0 have the same performance. I think all commits are safe, easy to grasp, except e30f67a is (maybe?) tricky.

With the performance now we basically hit the wall (with warm cache), in that the low ms 5, 6, 8 are spent in group_by_filename_and_update itself and not in the event listeners/view anymore.

braver · 2019-10-05T17:24:30Z

Nifty. Are you ready for a review here or want to do more work first?

kaste · 2019-10-05T18:51:52Z

This is good enough for review. Sure I can beautify and add some comments and better commit messages ... but I think this works and should be safe.

braver · 2019-10-05T20:24:53Z

Huh, I was under the impression that accessing settings via the ST API was essentially free. Maybe there is a trick to it, I'm not sure, it's been a while. Maybe load_settings is not free, whereas get any particular value will get it from memory directly. Not doing a load_settings if you don't need to sounds like a good idea anyway.

So, yeah, looks good indeed. I like how you improved the panel view, it's actually a lot simpler to read whereas e803a44 was a bit much.

I'll run with it for a few days, see if any errors turn up.

kaste · 2019-10-11T20:21:53Z

I maybe go for the extra mile and open separate PRs here. Looks like these are good improvements/patches with clear boundaries.

Also looks like that while we have on_lint_result which fires unconditionally we could just invent lint_result_changed. Usually a view doesn't need to update for the same result set. (This wasn't important for single file linters bc it is at least likely that the report changes for the file you're editing, but for multi file linters it is not so. You edit one file and the other ten files reported probably stay the same.) Don't need to do this immediately but it sounds reasonable.

Note: Queries like `views_into_file` are expensive on the worker.

We completely cache the settings in memory. This surprisingly boosts performance because if we access `load_settings` from the worker or any other thread we often pay a synchronization penalty of multiple ms. (Then again these lookups are zero cost.) The old implementation was also too sloppy because `has_changed` could only be called once in `on_update` per key. (On the second call it returned always `False`.)

This partially reverts "2cd32b7 Throttle panel repaints" We revert the throttler introduces in 2cd32b7, but keep the nice refactoring around the `draw` function.

Computing `filenames_per_window` is relatively slow, so we try to avoid that if we can.

Fixes #1671

kaste · 2019-10-15T12:27:03Z

No separate PR but I actually introduce 'lint_result_changed' bc that's really nicer and makes sense.

Please look at highlights_view bc I compute the gutter region slightly different. I don't see a difference visually. Maybe you know why we computed a 'line-region' for these icons. On my Sublime, it works with the 'normal' region just as well.

LGTM

braver · 2019-10-15T12:39:59Z

Maybe something about multiple errors on a line, or otherwise overlapping regions?

kaste · 2019-10-15T13:57:19Z

Good idea! The line comes from SL3 where the actual region was used to filter overlapping errors, but we just use the reported line for that now.

braver · 2019-10-15T14:08:20Z

I’ll try and put it through the ringer later today. Other than that the branch has been really solid for me.

braver · 2019-10-15T20:06:20Z

Ok, it looked good enough for what I tried now, but I need to try something more complicated with eslint. Will try to break it tomorrow and will report back.

braver · 2019-10-16T20:26:15Z

Ok, most of this looks and behaves well. I can't remember how to trigger overlapping multi-line errors. I seem to remember that happening with TSLint sometimes, but I haven't done any TypeScript for some time now and don't have any projects lying around.

That said, there is one problem: the tooltips on the gutter no longer work. On the regions themselves they're fine but this doesn't work anymore (screenshot taken on master while hovering over the line number):

kaste · 2019-10-17T08:45:22Z

You're really a good tester. 👏 👍

braver · 2019-10-17T09:14:53Z

🙇🏼‍♂️

- For the gutter marks/icons we don't need to compute a line region. There is no visual difference if we just use the region we already have. - For the highlights, we extract the 'offending_text' in the backend using the virtual view which is just much faster.

- Skip for non lintable views - Actually only emit when there were any changes - Broadcast 'filename' instead of 'view' Notable: the body of this function is basically zero cost in Sublime although it looks like a lot is happening here. After broadcasting, a normal panel redraw will occur (if a panel is open).

kaste · 2019-10-17T19:24:12Z

Fixed.

braver · 2019-10-17T20:16:50Z

Nice! Let's 🛥 this!

kaste · 2019-10-17T20:23:54Z

Workflow question: How do you checkout after I force-push?

braver · 2019-10-18T05:01:25Z

Well usually I just pull and then notice it doesn’t fast-forward. So then I switch to master, remove the branch, and checkout the branch again.

kaste · 2019-10-21T09:45:11Z

🙌

(After force-push, I reset --hard to upstream, but also not without this oh shit, I didn't want that moment.)

braver · 2019-10-21T10:29:16Z

Wanna ship this now as a minor release?

kaste · 2019-10-21T10:31:35Z

I'm on it right now

braver · 2019-10-21T12:54:42Z

🎉

kaste added 12 commits October 15, 2019 10:14

Add active views filename to the State for faster comparison

0a157ca

Draw status bar on main thread

c571567

Note: Queries like `views_into_file` are expensive on the worker.

Memoize computing error styles

2f50d35

Listen for settings_changed events in style.py

ad6b99e

Broadcast plugin_loaded and listen in style.py

eb85247

Listen for settings_changed in sublime_linter.py

576ddff

Do not throttle panel repaints

9ff95bb

This partially reverts "2cd32b7 Throttle panel repaints" We revert the throttler introduces in 2cd32b7, but keep the nice refactoring around the `draw` function.

Reorder conditional for performance

55e7be8

Computing `filenames_per_window` is relatively slow, so we try to avoid that if we can.

Do not draw panel/highlights if same result as before

816904c

Memoize format_error and create_path_dict

1be1968

Guard fill_panel against closed or hidden panel

ab0da7a

Fixes #1671

kaste force-pushed the boost-performance branch from b76c6b0 to 789007a Compare October 15, 2019 12:24

kaste force-pushed the boost-performance branch from 789007a to 0f0822c Compare October 16, 2019 08:16

kaste added 3 commits October 17, 2019 12:30

Do not rerender panel initially when starting the poller

5438924

kaste force-pushed the boost-performance branch from 0f0822c to 5438924 Compare October 17, 2019 10:31

kaste merged commit 9860713 into master Oct 21, 2019

kaste deleted the boost-performance branch October 21, 2019 09:42

kaste added this to the next milestone Oct 21, 2019

kaste mentioned this pull request Nov 6, 2019

Panel updates too often for multi-file reporters #1679

Closed

kaste added the 👀 Features and changes worth looking at label Nov 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Boost performance #1673

WIP: Boost performance #1673

kaste commented Oct 4, 2019 •

edited

kaste commented Oct 5, 2019

braver commented Oct 5, 2019

kaste commented Oct 5, 2019

braver commented Oct 5, 2019

kaste commented Oct 11, 2019

kaste commented Oct 15, 2019

braver commented Oct 15, 2019

kaste commented Oct 15, 2019

braver commented Oct 15, 2019

braver commented Oct 15, 2019

braver commented Oct 16, 2019

kaste commented Oct 17, 2019

braver commented Oct 17, 2019

kaste commented Oct 17, 2019

braver commented Oct 17, 2019

kaste commented Oct 17, 2019

braver commented Oct 18, 2019

kaste commented Oct 21, 2019

braver commented Oct 21, 2019

kaste commented Oct 21, 2019

braver commented Oct 21, 2019

WIP: Boost performance #1673

WIP: Boost performance #1673

Conversation

kaste commented Oct 4, 2019 • edited

kaste commented Oct 5, 2019

braver commented Oct 5, 2019

kaste commented Oct 5, 2019

braver commented Oct 5, 2019

kaste commented Oct 11, 2019

kaste commented Oct 15, 2019

braver commented Oct 15, 2019

kaste commented Oct 15, 2019

braver commented Oct 15, 2019

braver commented Oct 15, 2019

braver commented Oct 16, 2019

kaste commented Oct 17, 2019

braver commented Oct 17, 2019

kaste commented Oct 17, 2019

braver commented Oct 17, 2019

kaste commented Oct 17, 2019

braver commented Oct 18, 2019

kaste commented Oct 21, 2019

braver commented Oct 21, 2019

kaste commented Oct 21, 2019

braver commented Oct 21, 2019

kaste commented Oct 4, 2019 •

edited