validate_failures.py: Fix performance regression

2012-12-06  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>

	* testsuite-management/validate_failures.py
	(IsInterestingResult): Fix performance regression

---

Rephrasing the return statement was not a good idea, it regressed
tremendously ;)

==> LOG-before <==
real	0m22.696s
user	0m19.953s
sys	0m2.696s

==> LOG-r194182 <==
real	0m43.527s
user	0m38.346s
sys	0m5.092s

==> LOG-fixed <==
real	0m12.302s
user	0m10.841s
sys	0m1.432s

Sorry for that, OK for trunk?

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
---
 contrib/testsuite-management/validate_failures.py |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

On Thu, Dec 6, 2012 at 1:12 PM, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:

>  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.


OK with those changes.


Diego.

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.

On Thu, Dec 13, 2012 at 10:12 AM, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> 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?

That's for the attributes.  See the syntax at the top of the file.
One can add an attribute to a test.  For instance, mark it 'flaky' so
it's always ignored.  Or you can add an expiration date, to ignore it
until that timer elapses.

> 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

*shrug*

Performance is not really a problem with this script.



Diego.

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:
>
>>  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.
>
>
>OK with those changes.

Applied as r195811

thanks,