Refactor filter to not be initialized every time

We eagerly compile all the filters up front, then gather the
compiled filters into a DiagnosticFilter lazily, caching the
result to avoid garbage lists.

python/ycm/diagnostic_filter.py

@@ -24,20 +24,19 @@ standard_library.install_aliases()
 from builtins import *  # noqa
 
 from future.utils import iterkeys, iteritems
+from ycm import vimsupport
 import re
 
 
 class DiagnosticFilter( object ):
-  def __init__( self, config ):
+  def __init__( self, config_or_filters ):
-    self._filters = []
+    if isinstance( config_or_filters, list ):
+      self._filters = config_or_filters
+      print( 'NewFilter', config_or_filters)
 
-    for filter_type in iterkeys( config ):
+    else:
-      compiler = FILTER_COMPILERS.get( filter_type )
+      self._filters = _CompileFilters( config_or_filters )
-
+      print( 'CompileFilters', config_or_filters)
-      if compiler is not None:
-        for filter_config in _ListOf( config[ filter_type ] ):
-          compiledFilter = compiler( filter_config )
-          self._filters.append( compiledFilter )
 
 
   def IsAllowed( self, diagnostic ):
@@ -49,19 +48,59 @@ class DiagnosticFilter( object ):
     return True
 
 
-  @staticmethod
+  def SubsetForTypes( self, filetypes ):
-  def from_filetype( user_options, filetypes ):
+    """Return a sub-filter limited to the given filetypes"""
-    spec = {}
+    # NOTE: actually, this class is already filtered
-    all_filters = dict( user_options.get( 'filter_diagnostics', {} ) )
+    return self
-    for typeSpec, filterValue in iteritems( dict( all_filters ) ):
-      if typeSpec.find(',') != -1:
-        for filetype in typeSpec.split(','):
-          all_filters[ filetype ] = filterValue
 
+
+  @staticmethod
+  def CreateFromOptions( user_options ):
+    all_filters = dict( user_options.get( 'filter_diagnostics', {} ) )
+    compiled_by_type = {}
+    for type_spec, filter_value in iteritems( dict( all_filters ) ):
+      filetypes = [ type_spec ]
+      if type_spec.find( ',' ) != -1:
+        filetypes = type_spec.split( ',' )
+      for filetype in filetypes:
+        compiled_by_type[ filetype ] = _CompileFilters( filter_value )
+
+    return _MasterDiagnosticFilter( compiled_by_type )
+
+
+class _MasterDiagnosticFilter( object ):
+
+  def __init__( self, all_filters ):
+    self._all_filters = all_filters
+    self._cache = {}
+
+
+  def IsAllowed( self, diagnostic ):
+    # NOTE: in this class's implementation, we ask vimsupport for
+    #  the current filetypes and delegate automatically; it is probably,
+    #  more efficient, however, to call SubsetForTypes() and reuse
+    #  the returned DiagnosticFilter if it will be checked repeatedly.
+    filetypes = vimsupport.CurrentFiletypes()
+    return self.SubsetForTypes( filetypes ).IsAllowed( diagnostic )
+
+
+  def SubsetForTypes( self, filetypes ):
+    # check cache
+    cache_key = ','.join( filetypes )
+    cached = self._cache.get( cache_key )
+    if cached is not None:
+      return cached
+
+    # build a new DiagnosticFilter mergin all filters
+    #  for the provided filetypes
+    spec = []
     for filetype in filetypes:
-      type_specific = all_filters.get( filetype, {} )
+      type_specific = self._all_filters.get( filetype, [] )
-      spec = _Merge( spec, type_specific )
+      spec.extend( type_specific )
-    return DiagnosticFilter( spec )
+
+    new_filter = DiagnosticFilter( spec )
+    self._cache[ cache_key ] = new_filter
+    return new_filter
 
 
 def _ListOf( config_entry ):
@@ -74,13 +113,6 @@ def _ListOf( config_entry ):
   return [ config_entry ]
 
 
-def _Merge( into, other ):
-  for key in iterkeys( other ):
-    into[ key ] = _ListOf( into.get( key ) ) + _ListOf( other[ key ] )
-
-  return into
-
-
 def _CompileRegex( raw_regex ):
   pattern = re.compile( raw_regex, re.IGNORECASE )
 
@@ -104,3 +136,18 @@ def _CompileLevel( level ):
 
 FILTER_COMPILERS = { 'regex' : _CompileRegex,
                      'level' : _CompileLevel }
+
+
+def _CompileFilters( config ):
+  """Given a filter config dictionary, return a list of compiled filters"""
+  filters = []
+
+  for filter_type in iterkeys( config ):
+    compiler = FILTER_COMPILERS.get( filter_type )
+
+    if compiler is not None:
+      for filter_config in _ListOf( config[ filter_type ] ):
+        compiledFilter = compiler( filter_config )
+        filters.append( compiledFilter )
+
+  return filters

python/ycm/diagnostic_interface.py

@@ -33,6 +33,7 @@ import vim
 class DiagnosticInterface( object ):
   def __init__( self, user_options ):
     self._user_options = user_options
+    self._diag_filter = DiagnosticFilter.CreateFromOptions( user_options )
     # Line and column numbers are 1-based
     self._buffer_number_to_line_to_diags = defaultdict(
       lambda: defaultdict( list ) )
@@ -62,15 +63,13 @@ class DiagnosticInterface( object ):
 
   def PopulateLocationList( self, diags ):
     vimsupport.SetLocationList(
-      vimsupport.ConvertDiagnosticsToQfList( diags ) )
+      vimsupport.ConvertDiagnosticsToQfList(
+          self._ApplyDiagnosticFilter( diags ) ) )
 
 
   def UpdateWithNewDiagnostics( self, diags ):
-    diag_filter = DiagnosticFilter.from_filetype(
+    normalized_diags = [ _NormalizeDiagnostic( x ) for x in
-            self._user_options,
+            self._ApplyDiagnosticFilter( diags ) ]
-            vimsupport.CurrentFiletypes() )
-    normalized_diags = [ _NormalizeDiagnostic( x ) for x in diags
-            if diag_filter.IsAllowed( x ) ]
     self._buffer_number_to_line_to_diags = _ConvertDiagListToDict(
         normalized_diags )
 
@@ -86,6 +85,13 @@ class DiagnosticInterface( object ):
     if self._user_options[ 'always_populate_location_list' ]:
       self.PopulateLocationList( normalized_diags )
 
+
+  def _ApplyDiagnosticFilter( self, diags ):
+    filetypes = vimsupport.CurrentFiletypes()
+    diag_filter = self._diag_filter.SubsetForTypes( filetypes )
+    return filter( diag_filter.IsAllowed, diags )
+
+
   def _EchoDiagnosticForLine( self, line_num ):
     buffer_num = vim.current.buffer.number
     diags = self._buffer_number_to_line_to_diags[ buffer_num ][ line_num ]

python/ycm/tests/diagnostic_filter_test.py

@@ -24,6 +24,10 @@ standard_library.install_aliases()
 from builtins import *  # noqa
 
 from hamcrest import assert_that, equal_to
+
+from ycm.test_utils import MockVimModule
+MockVimModule()
+
 from ycm.diagnostic_filter import DiagnosticFilter
 
 
@@ -46,9 +50,13 @@ def _JavaFilter( config ):
   return { 'filter_diagnostics' : { 'java': config } }
 
 
+def _CreateFilterForTypes( opts, types ):
+  return DiagnosticFilter.CreateFromOptions( opts ).SubsetForTypes( types )
+
+
 def RegexFilter_test():
   opts = _JavaFilter( { 'regex' : 'taco' } )
-  f = DiagnosticFilter.from_filetype( opts, [ 'java' ] )
+  f = _CreateFilterForTypes( opts, [ 'java' ] )
 
   _assert_rejects( f, 'This is a Taco' )
   _assert_accepts( f, 'This is a Burrito' )
@@ -56,7 +64,7 @@ def RegexFilter_test():
 
 def RegexSingleList_test():
   opts = _JavaFilter( { 'regex' : [ 'taco' ] }  )
-  f = DiagnosticFilter.from_filetype( opts, [ 'java' ] )
+  f = _CreateFilterForTypes( opts, [ 'java' ] )
 
   _assert_rejects( f, 'This is a Taco' )
   _assert_accepts( f, 'This is a Burrito' )
@@ -64,15 +72,23 @@ def RegexSingleList_test():
 
 def RegexMultiList_test():
   opts = _JavaFilter( { 'regex' : [ 'taco', 'burrito' ] } )
-  f = DiagnosticFilter.from_filetype( opts, [ 'java' ] )
+  f = _CreateFilterForTypes( opts, [ 'java' ] )
 
   _assert_rejects( f, 'This is a Taco' )
   _assert_rejects( f, 'This is a Burrito' )
 
 
+def RegexNotFiltered_test():
+  opts = _JavaFilter( { 'regex' : 'taco' } )
+  f = _CreateFilterForTypes( opts, [ 'cs' ] )
+
+  _assert_accepts( f, 'This is a Taco' )
+  _assert_accepts( f, 'This is a Burrito' )
+
+
 def LevelWarnings_test():
   opts = _JavaFilter( { 'level' : 'warning' } )
-  f = DiagnosticFilter.from_filetype( opts, [ 'java' ] )
+  f = _CreateFilterForTypes( opts, [ 'java' ] )
 
   _assert_rejects( f, { 'text' : 'This is an unimportant taco',
                         'kind' : 'WARNING' } )
@@ -82,7 +98,7 @@ def LevelWarnings_test():
 
 def LevelErrors_test():
   opts = _JavaFilter( { 'level' : 'error' } )
-  f = DiagnosticFilter.from_filetype( opts, [ 'java' ] )
+  f = _CreateFilterForTypes( opts, [ 'java' ] )
 
   _assert_accepts( f, { 'text' : 'This is an IMPORTANT taco',
                         'kind' : 'WARNING' } )
@@ -94,7 +110,7 @@ def MultipleFilterTypesTypeTest_test():
 
   opts = _JavaFilter( { 'regex' : '.*taco.*',
                         'level' : 'warning' } )
-  f = DiagnosticFilter.from_filetype( opts, [ 'java' ] )
+  f = _CreateFilterForTypes( opts, [ 'java' ] )
 
   _assert_rejects( f, { 'text' : 'This is an unimportant taco',
                         'kind' : 'WARNING' } )
@@ -110,7 +126,7 @@ def MergeMultipleFiletypes_test():
     'java' : { 'regex' : '.*taco.*' },
     'xml'  : { 'regex' : '.*burrito.*' } } }
 
-  f = DiagnosticFilter.from_filetype( opts, [ 'java', 'xml' ] )
+  f = _CreateFilterForTypes( opts, [ 'java', 'xml' ] )
 
   _assert_rejects( f, 'This is a Taco' )
   _assert_rejects( f, 'This is a Burrito' )
@@ -122,7 +138,7 @@ def CommaSeparatedFiletypes_test():
   opts = { 'filter_diagnostics' : {
     'java,c,cs' : { 'regex' : '.*taco.*' } } }
 
-  f = DiagnosticFilter.from_filetype( opts, [ 'cs' ] )
+  f = _CreateFilterForTypes( opts, [ 'cs' ] )
 
   _assert_rejects( f, 'This is a Taco' )
   _assert_accepts( f, 'This is a Burrito' )