Re: [PATCH] validate_failures.py: Fix performance regression

[Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>]

On Fri, Dec 07, 2012 at 10:31:57AM -0500, Diego Novillo wrote:
>On Thu, Dec 6, 2012 at 1:12 PM, Bernhard Reutner-Fischer
><rep.dot.nop@gmail.com> wrote:

@@ -210,12 +211,12 @@ def IsInterestingResult(line):
   if '|' in line:
     (_, line) = line.split('|', 1)
     line = line.strip()
-  return any(line.startswith(result) for result in _VALID_TEST_RESULTS)
+  return bool(_VALID_TEST_RESULTS_REX.match(line))

I wonder why we care about '|' at all? Can you give an example where
this is of relevance?

Just asking because IIUC you throw away the beginning of the line until
the first pipe and try to match the remainder. Did you mean to do this
the other way round, throwing away the pipe and remainder instead?

I suspect that this function should just be
def IsInterestingResult(line):
  """Return True if line is one of the summary lines we care about."""
  return bool(_VALID_TEST_RESULTS_REX.match(line))

or, if there ever is a pipe in an interesting result
def IsInterestingResult(line):
  """Return True if line is one of the summary lines we care about."""
  if bool(_VALID_TEST_RESULTS_REX.match(line)):
    if '|' in line:
      (line, _) = line.split('|', 1)
      line = line.strip()
    return True
  return False

>
>>  def IsComment(line):
>>    """Return True if line is a comment."""
>> -  return line.startswith('#')
>> +  return bool(re.matches("#", line))
>
>startswith() is a better match here.
>
>>  def IsInclude(line):
>>    """Return True if line is an include of another file."""
>> -  return line.startswith("@include ")
>> +  return bool(re.matches("@include ", line))
>
>Likewise.
>
>>  def IsNegativeResult(line):
>>    """Return True if line should be removed from the expected results."""
>> -  return line.startswith("@remove ")
>> +  return bool(re.matches("@remove ", line))
>
>Likewise.

I dropped these 3.

TIA for clarification,
>
>
>OK with those changes.
>
>
>Diego.