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 when frequently modifying view settings #431

Open
randy3k opened this issue Aug 24, 2017 · 3 comments
Open

Performance issue when frequently modifying view settings #431

randy3k opened this issue Aug 24, 2017 · 3 comments
Labels
P: maybe Pending approval of low priority request. T: enhancement Enhancement.

Comments

@randy3k
Copy link

randy3k commented Aug 24, 2017

Performance issue when frequently modifying view settings

Recently I am working on a package with the following sublime_plugin.ViewEventListener

class FooBar(sublime_plugin.ViewEventListener):

    @classmethod
    def is_applicable(cls, settings):
        print("checked")
        return True

And I notice that the method is_applicable is called exactly two times when I move the cursor. I thus change
print("checked") to print(traceback.print_stack()) to see what functions are calling this frequently.

  File "bh_core in /Users/Randy/Dropbox/Sublime Text 3/Installed Packages/BracketHighlighter.sublime-package", line 1112, in payload
  File "bh_core in /Users/Randy/Dropbox/Sublime Text 3/Installed Packages/BracketHighlighter.sublime-package", line 443, in match
  File "/Applications/Sublime Text.app/Contents/MacOS/sublime.py", line 1149, in set
    sublime_api.settings_set(self.settings_id, key, value)
  File "/Applications/Sublime Text.app/Contents/MacOS/sublime_plugin.py", line 280, in <lambda>
    lambda: check_view_event_listeners(view))
  File "/Applications/Sublime Text.app/Contents/MacOS/sublime_plugin.py", line 267, in check_view_event_listeners
    want = is_view_event_listener_applicable(cls, view)
  File "/Applications/Sublime Text.app/Contents/MacOS/sublime_plugin.py", line 232, in is_view_event_listener_applicable
    if not cls.is_applicable(view.settings()):
  File "/Users/Randy/Dropbox/Sublime Text 3/Packages/LaTeXYZ/preview_math.py", line 14, in is_applicable
    print(traceback.print_stack())

It seems that every time I moved the cursor, BH changes the value of a view setting bracket_highlighter.busy twice, from False to True then False again. Each change triggers sublime_plugin.check_view_event_listeners and it then checks if all the ViewEventListener's are applicable to the current view.

There is no problem for doing so, except when a user has a considerable large amount of
ViewEventListener's and each move will take a non-negligible a time / computer power to finish.

Support Info

  • ST ver.: 3142
  • Platform: osx
  • Arch: x64
  • Plugin ver.: 2.25.0
  • Install via PC: True
  • mdpopups ver.: 2.1.1
  • backrefs ver.: 1.0.post1
  • markdown ver.: 2.6.6
  • pygments ver.: 2.1a0
  • jinja2 ver.: 2.8

Possible solution!?

BH should avoid using a view setting to flag its working status, may be a global variable?

@randy3k randy3k changed the title Performance issue for frequent modifying view settings Performance issue when frequently modifying view settings Aug 24, 2017
@facelessuser
Copy link
Owner

Thank you for the detailed issue, but please, please, please follow the template. The details are fine, but please always attach the version info stuff. I require this from everyone as I am trying to standardize these issues to encourage good issues. And when all examples exhibit the same pattern, it encourages others to do so as well. Thanks.

---end rant---

Each change triggers sublime_plugin.check_view_event_listeners and it then checks if all the ViewEventListener's are applicable to the current view.

This is definitely a new development since the introduction of ViewEventListener's. But it is possible this could be addressed. I'm not sure when I'll have the time to look into this though as I've been pretty busy recently. I'm a bit stretched by all the things I am maintaining and contributing to.

BH should avoid using a view setting to flag its working status, may be a global variable?

If you have a solution, I would encourage a pull request. Without looking into this, I can't say exactly how well simply using a generic global will work. Will I have to maintain a dictionary in which you index in per view id, and if so, require pruning of old view id's? Can I get away with a simple, single global variable? I'm not sure yet, and this will require further investigation. If this is becoming very troubling for you, a pull may get us there faster.

@randy3k
Copy link
Author

randy3k commented Aug 24, 2017

My bad. I have edited the post and tried my best to match the template.

I'm not sure when I'll have the time to look into this though as I've been pretty busy recently.

No problem. I appreciate you work to the ST community.

If you have a solution, I would encourage a pull request.

I am also not sure if a simple global will do the job properly. So I wanted to contact with you first to check if you have any insights. The situation is not that bad with a regular usage. But I could image it could be a problem if more packages are using ViewEventListener's given that BH is such a popular package. With that being said, I will look into this issue when I am free.

@facelessuser
Copy link
Owner

facelessuser commented Aug 24, 2017

My bad. I have edited the post and tried my best to match the template.

Thanks for being understanding 🙂 .

No problem. I appreciate you work to the ST community.

Thanks!

I am also not sure if a simple global will do the job properly. So I wanted to contact with you first to check if you have any insights. The situation is not that bad with a regular usage. But I could image it could be a problem if more packages are using ViewEventListener's given that BH is such a popular package. With that being said, I will look into this issue when I am free.

Off the top of my head I am unfortunately not sure. I haven't had to touch that area in quite some time. I'd have to refresh myself to see if this is an issue. BH uses a normal EventListener, which I think only one is created per Window? I don't know if you can run the risk of multiple view's running the BH matcher simultaneously or not. I was probably being cautious and used the view settings as it would tell me if I was currently busy in that view, and I wouldn't have to prune it since it is destroyed along with the view. It is possible that can be simplified.

I'll say this though, when executing a match, I do store that match info in the view setting as well, so this could still be an issue. I'm not sure if I have an easy solution for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: maybe Pending approval of low priority request. T: enhancement Enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants