owners.py: partial fix for owners-check perf regression

owners.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A database of OWNERS files.
 
 OWNERS files indicate who is allowed to approve changes in a specific directory
 (or who is allowed to make changes without needing approval of another OWNER).
 Note that all changes must still be reviewed by someone familiar with the code,
 so you may need approval from both an OWNER and a reviewer in many cases.
@@ skipping 163 matching lines @@
           if fnmatch.fnmatch(objname, owned_pattern):
             return True
       if self._should_stop_looking(objname):
         break
       objname = self.os_path.dirname(objname)
     return False
 
   def _enclosing_dir_with_owners(self, objname):
     """Returns the innermost enclosing directory that has an OWNERS file."""
     dirpath = objname
     while not self._owners_for(dirpath):

[ncarter (slow), 2016/08/30 22:51:27]

I wonder if this check needs something like what happened with
load_data_needed_for().

It seems odd to ever call _owners_for() on a dirname (versus a full file path),
since the glob rules in OWNERS files are meant to pertain to files, not dirs.

       if self._should_stop_looking(dirpath):
         break
       dirpath = self.os_path.dirname(dirpath)
     return dirpath
 
   def load_data_needed_for(self, files):
     for f in files:
+      loaded_dirs = set()
       dirpath = self.os_path.dirname(f)
-      while not self._owners_for(dirpath):
+      while dirpath not in loaded_dirs:

[dtu, 2016/08/31 02:35:46]

Is loaded_dirs supposed to be at the top-level? I think as written it runs until
loaded_dirs becomes '', so this is the same as doing "while dirpath:"

"while dirpath:" would still be correct here. If it's faster than the current
check, then that's an easy change to make for both this function and
_enclosing_dir_with_owners.

         self._read_owners(self.os_path.join(dirpath, 'OWNERS'))
+        loaded_dirs.add(dirpath)
         if self._should_stop_looking(dirpath):
           break
         dirpath = self.os_path.dirname(dirpath)
 
   def _should_stop_looking(self, objname):
     return any(fnmatch.fnmatch(objname, stop_looking)
                for stop_looking in self._stop_looking)
 
   def _owners_for(self, objname):
     obj_owners = set()
@@ skipping 157 matching lines @@
   @staticmethod
   def lowest_cost_owner(all_possible_owners, dirs):
     total_costs_by_owner = Database.total_costs_by_owner(all_possible_owners,
                                                          dirs)
     # Return the lowest cost owner. In the case of a tie, pick one randomly.
     lowest_cost = min(total_costs_by_owner.itervalues())
     lowest_cost_owners = filter(
         lambda owner: total_costs_by_owner[owner] == lowest_cost,
         total_costs_by_owner)
     return random.Random().choice(lowest_cost_owners)