Bram, we _think_ the overhead is calling prop_list for every line, and that doing that in one call spanning the whole buffer would be more efficient. (currently we're doing a request per line, filtering the result for the types that start with Ycm, then adding them to a python list).
That said, it would be certainly useful for our use case to be able to only return props of a given list of types as well. That allows us to say "return us all the properties of <these types> in <this range> of the buffer". I'd suspect that for _most_ plugin authors that they would only be interested in props that they created themselves. I can imagine scenarios where PluginA looks at PluginB's text properties, but would expect that to be the rare case rather than the norm. I think that would be relatively faster when scanning the memlines than doing a request per-line and filtering in vimscript land. > Whatever solution we pick, it is likely still slow for a very long file. Yes, this is true. In fact, we already disable this functionality on very large files (the inefficiency was actually discovered because of a bug in that!); we're now more thinking about efficiency on moderate usage as that affects more users of course. > Have you considered only dealing with visible lines? I don't think we had considered this. Certainly this adds quite a lot of complexity - we would have to listen for scrolling events and update the properties as the user scrolls, handling there being multiple windows per buffer etc. I had previously assumed this wasn't even possible (is there an autocmd for "window scrolled"?), but I admit I haven't thought hard about it. On Sunday, November 14, 2021 at 10:32:36 PM UTC Bram Moolenaar wrote: > > > [prop_list_profile.txt]( > https://github.com/vim/vim/files/7527900/prop_list_profile.txt) > > #### The problem > > > > If a plugin wants to grab all of the previously placed text properties > that the same plugin has placed, the current `prop_list` requires some work: > > > > 1. Loop over the lines of the buffer. > > 2. Grab all the text properties from each line and aggregate in some > list. > > 3. Filter out all properties that are not produced by the plugin. A > plugin might need to grab two types of properties, so making vim filter by > `type` isn't perfect either. > > > > This results in a ton of vimscript API calls and is a similar thing to > what lead to https://github.com/vim/vim/issues/8675. > > > > Basically, what a plugin is running is this: > > > > ```viml > > function! g:UpdateMatches() > > let properties = [] > > for line in range( line('$') ) > > call extend(properties, filter(prop_list( line + 1, { 'bufnr': 1 } ), > 'v:val[ "type" ][ :3 ] ==# "Ycm"' )) > > endfor > > endfunction > > ``` > > The profile is here: > https://gist.github.com/bstaletic/84c10b75be93c99f321a27af73278a8d > > > > ### The neovim API > > > > I'm not the biggest fan of neovim, but its API here is really nice. > > > > Each plugin gets a unique namespace ID. Then, when getting (neovim's > equivalent of) all the properties in a buffer, besides the buffer number, > the ID is passed as well. Introducing this namespace ID thing to text > property APIs would be perfect. Vim's `sign_foo()` family of functions > already has the `group` thing. > > > > #### Alternative > > > > A simpler solution is just introducing something like > `prop_list_range()`, where not only a single line would be passed, but > rather a range of lines. That way plugins could request all text properties > from line N to line M. It would still leave filtering to the users. > > > > Note that the two ideas aren't exactly exclusive. > > > > #### Context > > > > I'm a maintainer of YouCompleteMe. The way YCM updates text properties > for a specific buffer is basically described in the "problem" section. In > more detail: > > > > - We define two property types: > https://github.com/ycm-core/YouCompleteMe/blob/master/autoload/youcompleteme.vim#L418-L433 > > - When we update the diagnostics, we do it like this: > > - [First, we grab all placed properties]( > https://github.com/ycm-core/YouCompleteMe/blob/master/python/ycm/diagnostic_interface.py#L133 > ) > > - [For each new diagnostic]( > https://github.com/ycm-core/YouCompleteMe/blob/master/python/ycm/diagnostic_interface.py#L137 > ) > > - [if the exact same has already been placed, just skip]( > https://github.com/ycm-core/YouCompleteMe/blob/master/python/ycm/diagnostic_interface.py#L149 > ) > > - [else, place a new one]( > https://github.com/ycm-core/YouCompleteMe/blob/master/python/ycm/diagnostic_interface.py#L151-L156 > ) > > - Could be optimized with `prop_add_list()`. In the TODO list. > > - Once we have skipped all the ones we want to keep and added the new > ones, what remains is [to remove the stale ones]( > https://github.com/ycm-core/YouCompleteMe/blob/master/python/ycm/diagnostic_interface.py#L158-L159 > ) > > - There's no batch remove of properties, but *so far* that hasn't been a > problem. > > > > The algorithm is made that way, because in common cases it's faster than > removing all old and then placing all new - i.e. paying attention to > overlap between the two sets is beneficial. > > > > > > > > But I digress. The problem is that call to > `vimsupport.GetTextProperties( bufnr )`. It's defined here: > > > > > https://github.com/ycm-core/YouCompleteMe/blob/master/python/ycm/vimsupport.py#L213-L226 > > > > Doing that on an extra long file is very inefficient. For a file that > > is 1'010'000 lines long, with no props either placed, or to place > > (i.e. all work is in `GetTextProperties()`), it takes ~1 minute on my > > machine. > > What is the overhead caused by? If it's calling prop_list() for every > line, we could add an entry in the {props} argument "linecount". Making > it very high would return properties for all lines from {lnum} to the > last line. The returned list would have "lnum" added in each entry. > > If the overhead is caused by returning lots of properties that are not > related to the plugin, they are dropped when filtering, we could add a > pattern to match with "type". If you want to find two types you can use > 'name_one\|name_two". > > Whatever solution we pick, it is likely still slow for a very long file. > Have you considered only dealing with visible lines? > > If you need a unique ID for a plugin, use the script ID of the plugin > file. It is unique, and when the plugin is reloaded it will remain the > same. > > -- > WOMAN: Dennis, there's some lovely filth down here. Oh -- how d'you do? > ARTHUR: How do you do, good lady. I am Arthur, King of the Britons. > Who's castle is that? > WOMAN: King of the who? > The Quest for the Holy Grail (Monty Python) > > /// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\ > /// \\\ > \\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// > \\\ help me help AIDS victims -- http://ICCF-Holland.org /// > > — > You are receiving this because you commented. > > > Reply to this email directly, view it on GitHub > <https://github.com/vim/vim/issues/9128#issuecomment-968375908>. > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > > > -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/vim_dev/fc6ef6dd-3d08-4d5b-9c28-71c233987been%40googlegroups.com.
