Auto merge of #2871 - micbou:optimize-request-building, r=bstaletic

[READY] Optimize request building

This PR significantly reduces the time spent to build the request when there are a lot of buffers. This is done by:
 - using the `options` property on the buffer object to get the `mod` variable instead of evaluating `getbufvar`;
 - not computing the buffer filepath if the buffer is not modified;
 - passing the number of the unloaded buffer instead of its filepath on the `BufferUnload` event. Getting the Python buffer object from its number is easier than from its filepath.

Here are the times spent to edit a new empty buffer (i.e. two event notifications `BufferVisit` and `FileReadyToParse`) for different numbers of buffers already open in the editor before and after the changes:
<table>
  <tr>
    <th>Number of buffers open</th>
    <th>Before (ms)</th>
    <th>After (ms)</th>
  </tr>
  <tr>
    <td>1</td>
    <td>1.2</td>
    <td>1.0</td>
  </tr>
  <tr>
    <td>10</td>
    <td>1.2</td>
    <td>1.0</td>
  </tr>
  <tr>
    <td>100</td>
    <td>2.4</td>
    <td>1.1</td>
  </tr>
  <tr>
    <td>1000</td>
    <td>13.5</td>
    <td>2.2</td>
  </tr>
  <tr>
    <td>10000</td>
    <td>139.8</td>
    <td>17.3</td>
  </tr>
</table>

Results obtained with the `prof.py` script from [that branch](https://github.com/micbou/YouCompleteMe/tree/profiling-request-building).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2871)
<!-- Reviewable:end -->

autoload/youcompleteme.vim

@@ -1,4 +1,4 @@
-" Copyright (C) 2011, 2012  Google Inc.
+" Copyright (C) 2011-2018 YouCompleteMe contributors
 "
 " This file is part of YouCompleteMe.
 "
@@ -475,13 +475,12 @@ endfunction
 function! s:OnBufferUnload()
   " Expanding <abuf> returns the unloaded buffer number as a string but we want
   " it as a true number for the getbufvar function.
-  if !s:AllowedToCompleteInBuffer( str2nr( expand( '<abuf>' ) ) )
+  let buffer_number = str2nr( expand( '<abuf>' ) )
+  if !s:AllowedToCompleteInBuffer( buffer_number )
     return
   endif
 
-  let deleted_buffer_file = expand( '<afile>:p' )
+  exec s:python_command "ycm_state.OnBufferUnload( " . buffer_number . " )"
-  exec s:python_command "ycm_state.OnBufferUnload(" .
-        \ "vim.eval( 'deleted_buffer_file' ) )"
 endfunction
 
 

python/ycm/client/base_request.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2013  Google Inc.
+# Copyright (C) 2013-2018 YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -25,6 +25,7 @@ from builtins import *  # noqa
 import contextlib
 import logging
 import json
+import vim
 from future.utils import native
 from base64 import b64decode, b64encode
 from ycm import vimsupport
@@ -152,22 +153,26 @@ class BaseRequest( object ):
   hmac_secret = ''
 
 
-def BuildRequestData( filepath = None ):
+def BuildRequestData( buffer_number = None ):
-  """Build request for the current buffer or the buffer corresponding to
+  """Build request for the current buffer or the buffer with number
-  |filepath| if specified."""
+  |buffer_number| if specified."""
-  current_filepath = vimsupport.GetCurrentBufferFilepath()
   working_dir = GetCurrentDirectory()
+  current_buffer = vim.current.buffer
 
-  if filepath and current_filepath != filepath:
+  if buffer_number and current_buffer.number != buffer_number:
     # Cursor position is irrelevant when filepath is not the current buffer.
+    buffer_object = vim.buffers[ buffer_number ]
+    filepath = vimsupport.GetBufferFilepath( buffer_object )
     return {
       'filepath': filepath,
       'line_num': 1,
       'column_num': 1,
       'working_dir': working_dir,
-      'file_data': vimsupport.GetUnsavedAndSpecifiedBufferData( filepath )
+      'file_data': vimsupport.GetUnsavedAndSpecifiedBufferData( buffer_object,
+                                                                filepath )
     }
 
+  current_filepath = vimsupport.GetBufferFilepath( current_buffer )
   line, column = vimsupport.CurrentLineAndColumn()
 
   return {
@@ -175,7 +180,8 @@ def BuildRequestData( filepath = None ):
     'line_num': line + 1,
     'column_num': column + 1,
     'working_dir': working_dir,
-    'file_data': vimsupport.GetUnsavedAndSpecifiedBufferData( current_filepath )
+    'file_data': vimsupport.GetUnsavedAndSpecifiedBufferData( current_buffer,
+                                                              current_filepath )
   }
 
 

python/ycm/client/event_notification.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2013  Google Inc.
+# Copyright (C) 2013-2018 YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -27,17 +27,17 @@ from ycm.client.base_request import ( BaseRequest, BuildRequestData,
 
 
 class EventNotification( BaseRequest ):
-  def __init__( self, event_name, filepath = None, extra_data = None ):
+  def __init__( self, event_name, buffer_number = None, extra_data = None ):
     super( EventNotification, self ).__init__()
     self._event_name = event_name
-    self._filepath = filepath
+    self._buffer_number = buffer_number
     self._extra_data = extra_data
     self._response_future = None
     self._cached_response = None
 
 
   def Start( self ):
-    request_data = BuildRequestData( self._filepath )
+    request_data = BuildRequestData( self._buffer_number )
     if self._extra_data:
       request_data.update( self._extra_data )
     request_data[ 'event_name' ] = self._event_name
@@ -64,7 +64,7 @@ class EventNotification( BaseRequest ):
 
 
 def SendEventNotificationAsync( event_name,
-                                filepath = None,
+                                buffer_number = None,
                                 extra_data = None ):
-  event = EventNotification( event_name, filepath, extra_data )
+  event = EventNotification( event_name, buffer_number, extra_data )
   event.Start()

python/ycm/tests/client/base_request_test.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2017 YouCompleteMe Contributors
+# Copyright (C) 2017-2018 YouCompleteMe Contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -22,7 +22,7 @@ from __future__ import absolute_import
 # Not installing aliases from python-future; it's unreliable and slow.
 from builtins import *  # noqa
 
-from ycm.tests.test_utils import MockVimModule
+from ycm.tests.test_utils import MockVimBuffers, MockVimModule, VimBuffer
 MockVimModule()
 
 from hamcrest import assert_that, has_entry
@@ -32,14 +32,16 @@ from ycm.client.base_request import BuildRequestData
 
 @patch( 'ycm.client.base_request.GetCurrentDirectory',
         return_value = '/some/dir' )
-@patch( 'ycm.vimsupport.CurrentLineAndColumn', return_value = ( 1, 1 ) )
 def BuildRequestData_AddWorkingDir_test( *args ):
-  assert_that( BuildRequestData(), has_entry( 'working_dir', '/some/dir' ) )
+  current_buffer = VimBuffer( 'foo' )
+  with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 1 ) ):
+    assert_that( BuildRequestData(), has_entry( 'working_dir', '/some/dir' ) )
 
 
 @patch( 'ycm.client.base_request.GetCurrentDirectory',
         return_value = '/some/dir' )
-@patch( 'ycm.vimsupport.CurrentLineAndColumn', return_value = ( 1, 1 ) )
 def BuildRequestData_AddWorkingDirWithFileName_test( *args ):
-  assert_that( BuildRequestData( 'foo' ),
+  current_buffer = VimBuffer( 'foo' )
-               has_entry( 'working_dir', '/some/dir' ) )
+  with MockVimBuffers( [ current_buffer ], current_buffer, ( 1, 1 ) ):
+    assert_that( BuildRequestData( current_buffer.number ),
+                 has_entry( 'working_dir', '/some/dir' ) )

python/ycm/tests/event_notification_test.py

@@ -1,6 +1,6 @@
 # coding: utf-8
 #
-# Copyright (C) 2015-2016 YouCompleteMe contributors
+# Copyright (C) 2015-2018 YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -25,8 +25,7 @@ from __future__ import absolute_import
 from builtins import *  # noqa
 
 from ycm.tests.test_utils import ( CurrentWorkingDirectory, ExtendedMock,
-                                   MockVimBuffers, MockVimModule, VimBuffer,
+                                   MockVimBuffers, MockVimModule, VimBuffer )
-                                   ToBytesOnPY2 )
 MockVimModule()
 
 import contextlib
@@ -398,7 +397,6 @@ def EventNotification_BufferVisit_BuildRequestForCurrentAndUnsavedBuffers_test(
                               contents = [ 'current_buffer_contents' ],
                               filetype = 'some_filetype',
                               modified = False )
-
   modified_buffer_file = os.path.realpath( 'modified_buffer' )
   modified_buffer = VimBuffer( name = modified_buffer_file,
                                number = 2,
@@ -465,7 +463,7 @@ def EventNotification_BufferUnload_BuildRequestForDeletedAndUnsavedBuffers_test(
   with patch( 'ycm.client.event_notification.EventNotification.'
               'PostDataToHandlerAsync' ) as post_data_to_handler_async:
     with MockVimBuffers( [ current_buffer, deleted_buffer ], current_buffer ):
-      ycm.OnBufferUnload( ToBytesOnPY2( deleted_buffer_file ) )
+      ycm.OnBufferUnload( deleted_buffer.number )
 
   assert_that(
     # Positional arguments passed to PostDataToHandlerAsync.

python/ycm/tests/test_utils.py

@@ -1,5 +1,4 @@
-# Copyright (C) 2011-2012 Google Inc.
+# Copyright (C) 2011-2018 YouCompleteMe contributors
-#               2016      YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -272,6 +271,10 @@ class VimBuffer( object ):
     self.omnifunc = omnifunc
     self.omnifunc_name = omnifunc.__name__ if omnifunc else ''
     self.changedtick = 1
+    self.options = {
+     'mod': modified,
+     'bh': bufhidden
+    }
 
 
   def __getitem__( self, index ):
@@ -292,6 +295,27 @@ class VimBuffer( object ):
     return [ ToUnicode( x ) for x in self.contents ]
 
 
+class VimBuffers( object ):
+  """An object that looks like a vim.buffers object."""
+
+  def __init__( self, *buffers ):
+    """Arguments are VimBuffer objects."""
+    self._buffers = buffers
+
+
+  def __getitem__( self, number ):
+    """Emulates vim.buffers[ number ]"""
+    for buffer_object in self._buffers:
+      if number == buffer_object.number:
+        return buffer_object
+    raise KeyError( number )
+
+
+  def __iter__( self ):
+    """Emulates for loop on vim.buffers"""
+    return iter( self._buffers )
+
+
 class VimMatch( object ):
 
   def __init__( self, group, pattern ):
@@ -326,7 +350,7 @@ def MockVimBuffers( buffers, current_buffer, cursor_position = ( 1, 1 ) ):
 
   line = current_buffer.contents[ cursor_position[ 0 ] - 1 ]
 
-  with patch( 'vim.buffers', buffers ):
+  with patch( 'vim.buffers', VimBuffers( *buffers ) ):
     with patch( 'vim.current.buffer', current_buffer ):
       with patch( 'vim.current.window.cursor', cursor_position ):
         with patch( 'vim.current.line', line ):

python/ycm/tests/vimsupport_test.py

@@ -1,6 +1,6 @@
 # coding: utf-8
 #
-# Copyright (C) 2015-2016 YouCompleteMe contributors
+# Copyright (C) 2015-2018 YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -1434,7 +1434,8 @@ def GetUnsavedAndSpecifiedBufferData_EncodedUnicodeCharsInBuffers_test():
   vim_buffer = VimBuffer( filepath, contents = contents )
 
   with patch( 'vim.buffers', [ vim_buffer ] ):
-    assert_that( vimsupport.GetUnsavedAndSpecifiedBufferData( filepath ),
+    assert_that( vimsupport.GetUnsavedAndSpecifiedBufferData( vim_buffer,
+                                                              filepath ),
                  has_entry( filepath,
                             has_entry( u'contents', u'abc\nfДa\n' ) ) )
 

python/ycm/vimsupport.py

@@ -1,5 +1,4 @@
-# Copyright (C) 2011-2012 Google Inc.
+# Copyright (C) 2011-2018 YouCompleteMe contributors
-#               2016      YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -100,41 +99,33 @@ def TextBeforeCursor():
   return ToUnicode( vim.current.line[ :CurrentColumn() ] )
 
 
-# Note the difference between buffer OPTIONS and VARIABLES; the two are not
-# the same.
-def GetBufferOption( buffer_object, option ):
-  # NOTE: We used to check for the 'options' property on the buffer_object which
-  # is available in recent versions of Vim and would then use:
-  #
-  #   buffer_object.options[ option ]
-  #
-  # to read the value, BUT this caused annoying flickering when the
-  # buffer_object was a hidden buffer (with option = 'ft'). This was all due to
-  # a Vim bug. Until this is fixed, we won't use it.
-
-  to_eval = 'getbufvar({0}, "&{1}")'.format( buffer_object.number, option )
-  return GetVariableValue( to_eval )
-
-
 def BufferModified( buffer_object ):
-  return bool( int( GetBufferOption( buffer_object, 'mod' ) ) )
+  return buffer_object.options[ 'mod' ]
 
 
-def GetUnsavedAndSpecifiedBufferData( including_filepath ):
+def GetBufferData( buffer_object ):
+  return {
+    # Add a newline to match what gets saved to disk. See #1455 for details.
+    'contents': JoinLinesAsUnicode( buffer_object ) + '\n',
+    'filetypes': FiletypesForBuffer( buffer_object )
+  }
+
+
+def GetUnsavedAndSpecifiedBufferData( included_buffer, included_filepath ):
   """Build part of the request containing the contents and filetypes of all
-  dirty buffers as well as the buffer with filepath |including_filepath|."""
+  dirty buffers as well as the buffer |included_buffer| with its filepath
-  buffers_data = {}
+  |included_filepath|."""
+  buffers_data = { included_filepath: GetBufferData( included_buffer ) }
+
   for buffer_object in vim.buffers:
-    buffer_filepath = GetBufferFilepath( buffer_object )
+    if not BufferModified( buffer_object ):
-    if not ( BufferModified( buffer_object ) or
-             buffer_filepath == including_filepath ):
       continue
 
-    buffers_data[ buffer_filepath ] = {
+    filepath = GetBufferFilepath( buffer_object )
-      # Add a newline to match what gets saved to disk. See #1455 for details.
+    if filepath in buffers_data:
-      'contents': JoinLinesAsUnicode( buffer_object ) + '\n',
+      continue
-      'filetypes': FiletypesForBuffer( buffer_object )
+
-    }
+    buffers_data[ filepath ] = GetBufferData( buffer_object )
 
   return buffers_data
 
@@ -362,7 +353,7 @@ def VimExpressionToPythonType( vim_expression ):
 
 
 def HiddenEnabled( buffer_object ):
-  if GetBufferOption( buffer_object, 'bh' ) == "hide":
+  if buffer_object.options[ 'bh' ] == "hide":
     return True
   return GetBoolValue( '&hidden' )
 
@@ -601,7 +592,14 @@ def GetBufferFiletypes( bufnr ):
 def FiletypesForBuffer( buffer_object ):
   # NOTE: Getting &ft for other buffers only works when the buffer has been
   # visited by the user at least once, which is true for modified buffers
-  return ToUnicode( GetBufferOption( buffer_object, 'ft' ) ).split( '.' )
+
+  # We don't use
+  #
+  #   buffer_object.options[ 'ft' ]
+  #
+  # to get the filetypes because this causes annoying flickering when the buffer
+  # is hidden.
+  return GetBufferFiletypes( buffer_object.number )
 
 
 def VariableExists( variable ):

python/ycm/youcompleteme.py

@@ -1,5 +1,4 @@
-# Copyright (C) 2011-2012 Google Inc.
+# Copyright (C) 2011-2018 YouCompleteMe contributors
-#               2016-2017 YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -381,10 +380,8 @@ class YouCompleteMe( object ):
     self.CurrentBuffer().SendParseRequest( extra_data )
 
 
-  def OnBufferUnload( self, deleted_buffer_file ):
+  def OnBufferUnload( self, deleted_buffer_number ):
-    SendEventNotificationAsync(
+    SendEventNotificationAsync( 'BufferUnload', deleted_buffer_number )
-        'BufferUnload',
-        filepath = utils.ToUnicode( deleted_buffer_file ) )
 
 
   def OnBufferVisit( self ):