SublimeText / sublime_lib

Utility library for frequently used functionality in Sublime Text and convenience functions or classes
https://sublimetext.github.io/sublime_lib
MIT License
52 stars 4 forks source link

Add an event listener to create an API to tell whether a view is cloned or not #127

Open evandrocoan opened 4 years ago

evandrocoan commented 4 years ago

The View.is_primary() only tells if the view is the "primary". If the view is the "primary", it does not tell whether it has multiple views into the underlying buffer.

Some years ago, I wrote this code into my fork of BufferScroll: https://github.com/evandrocoan/BufferScroll/blob/master/BufferScroll.py#L105-L117

And today, I need to write the same code for my new package: https://github.com/evandrocoan/SkipCloseForClonedViews/blob/master/skip_close_for_cloned_views.py#L5-L70

Which implements this feature asked on Core:

  1. https://github.com/SublimeTextIssues/Core/issues/2903 Skip save (ask) when closing a cloned view

Nowadays, I am worried about performance, and I am running this is_cloned_view() very often. Then, for my new Package, I wrote a optimized version which caches whether a given buffer has several views into the same buffer.

How such peculiar API is_cloned(view) which requires to extend the sublime_plugin.EventListener be distributed and shared between several packages? Could it be added to sublime_lib?

Something very similar was already requested to be added into Sublime Text Core:

  1. https://github.com/SublimeTextIssues/Core/issues/11 "View.clones()" addition

Using that API, it would only require me to call len( view.clones() ) > 1 to know where some view is cloned, i.e., has clones, i.e., has several views into the same buffer. Then, optimally when integrating my implementation into sublime_lib, my original implementation could be extended to support this feature of returning all clones instead of only saying some view has clones or not.

Thom1729 commented 4 years ago

The easy way to do this is:

len([
    v for v in view.window().views()
    if v.buffer_id() == view.buffer_id()
]) > 1

Or, if you want to examine all windows:

len([
    v for w in sublime.windows() for v in w.views()
    if v.buffer_id() == view.buffer_id()
]) > 1

In SkipCloseForClonedViews, this should only be run once when the user manually tries to close a view. It is difficult to imagine this causing a performance problem. On the other hand, maintaining a cache requires running an event listener whenever a view is opened or closed, which seems like it should involve at least as much overhead as running the Python expression above. Have you measured the relative performance of the approaches?

evandrocoan commented 4 years ago

I did not timed any code to choose this approach. It just seemed faster cache the results instead of computing them every time I need them, because I already need them every time I am closing a view:

  1. If I cache the results, my computation costs are just some simple/non-expensive O(1) calls when some view is created or deleted.
  2. If I do not cache the results, every time some view is closed I would be required to do a loop throughout all opened views and windows to calculate whether the view I am closing is cloned or not. This should cost about O(n^2) when I am closing a window with all its views on it. For example, if I have a window with 10.000 views and I close the whole window:
    1. When the first view is closed, it would have to loop through 9.999 views.
    2. When the second view is closed, it would have to loop through 9.998 views.
    3. ... sum from 1 to 10000 computing steps is 50.000.000 and 10.000^2 is 100.000.000 then the complexity is about O(n^2)

But, if I am caching the results it should not be expensive to close a window with 10.000 views because each time I just need to perform few constant O(1) operations, independent of how many view there are open.

On the other hand, maintaining a cache requires running an event listener whenever a view is opened or closed, which seems like it should involve at least as much overhead as running the Python expression above.

As I need the results every time I close a view, then it seems more reasonable to cache the results as should be more expensive to loop through all views every time I close a view. For comparison:

  1. Without caching
    def is_cloned(self, view):
        return \
            len([
                    v for w in sublime.windows() for v in w.views()
                    if v.buffer_id() == view.buffer_id()
                ]) > 1
  2. With caching

    cloned_buffers_ids = {}
    
    def is_cloned(self, view):
        return self.cloned_buffers_ids[view.buffer_id()] > 1
    
    def on_load(self, view):
        self.on_new( view )
    
    def on_new(self, view):
        self.cloned_buffers_ids[view.buffer_id()] = 1
    
    def on_clone(self, view):
        self.cloned_buffers_ids[view.buffer_id()] += 1
    
    def on_close(self, view):
        buffer_id = view.buffer_id()
        self.cloned_buffers_ids[buffer_id] -= 1
    
        if self.cloned_buffers_ids[buffer_id] < 1:
            del self.cloned_buffers_ids[buffer_id]
Thom1729 commented 4 years ago

if I have a window with 10.000 views

Have you tried this? I did just to see what would happen, and the performance seemed nonlinear anyway. I'm not sure that it's practical or realistic for users to keep ten thousand tabs open in one window.

Myself, if I have enough tabs that I can't see all of the tab names, then I feel like I'm not getting much use out of having them open. This means a rough limit of about 20 tabs per window. Given the simplicity of the brute-force check, I would not expect the difference between 20 and 400 executions to be noticeable. What kinds of tab counts do you run into in your workflow?

FichteFoll commented 4 years ago

I just want to interject that I'd be extremely hesitant on adding anything to sublime_lib that interfaces with ST directly, e.g. a sublime_plugin.EventListener. Because dependencies aren't packages, we would need to manually patch this into sublime_plugin's global mappings and I'd rather not do that. When we then think about how we could export the functionality for some other package to make ST register it there, we'd end up having the listeners as many times as plugins use them, so in the end nothing is gained.

Edit: Oh, add to that that dependencies aren't even loaded unless imported or a loader.py was specified, and this certainly isn't something I'd be looking forward to, to say the least.

evandrocoan commented 4 years ago

Have you tried this?

I do that when I do Ctrl+Shift+F and do a Replace All for something in project with thousands of files and the replacing happens in thousands of files. In that case, I would use some option as close all tabs and just close the window. In my usual workflow, I got 5~15 tabs per workspace or panel.

dependencies aren't even loaded unless imported or a loader.py

On this case, if no package imported the dependency, there is no need to keep the cache, i.e., register the event listener.

One solution could be create a function called register_event_listeners(), then, if some package would like to use the cached values, they should call this function which would register the event listeners to keep the cloned views cache up to date.

we'd end up having the listeners as many times as plugins use them, so in the end nothing is gained.

I did not understand this part. If the event listener to keep the cache of cloned views are registered, it would happen only once for all packages which are using the cached results.

Thom1729 commented 4 years ago

How long does it take Sublime to close thousands of file tabs on your machine? When I tested, it took about 40 seconds to create ten thousand tabs, and about the same to close the window.

FichteFoll commented 4 years ago

I did not understand this part. If the event listener to keep the cache of cloned views are registered, it would happen only once for all packages which are using the cached results.

True, if the API is to register the listener once and store that into globals that would work. Not the greatest solution but one that would at least not run into the problems I mentioned. My first point still stands, however. And that's of course not even considering whether this even should be cached in the first place.

evandrocoan commented 4 years ago

How long does it take Sublime to close thousands of file tabs on your machine?

How did did you created these tabs?

I ran a find and replace on 10.036 files and it took about a hour to finish and 4:10 seconds to close all the tabs. (Note here I am using a vanilla install of Sublime Text without any third part code running and I am including the time to save all these files modifications with File - Save All).