try precompiling regexps to speed up bench_graph_svg.py

try precompiling regexps to speed up bench_graph_svg.py
Committed: https://code.google.com/p/skia/source/detail?r=7741

[epoger, 2013-02-14 15:43:04]

I haven't named a reviewer because so far, this actually isn't worth committing.
 It takes the same amount of time (39 seconds) to run the self-test with and
without this change.

As noted in
http://stackoverflow.com/questions/452104/is-it-worth-using-pythons-re-compile ,
it looks like manually compiling the regex (instead of letting it get compiled
on the fly) is not particularly helpful.

I may try some other speedups, and I encourage anyone else to do the same...

[epoger, 2013-02-14 16:11:21]

Ready for review at patchset 2.

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py
File bench/bench_util.py (right):

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode33
bench/bench_util.py:33: SETTING_RE_COMPILED = re.compile(SETTING_RE)
Surprisingly, this still isn't any faster than it was before!

But at this point, I think it's worth committing, just to make it clear that all
of the regexes in here are constants.  None of them use different patterns as
the code runs.

[benchen, 2013-02-14 17:43:31]

LGTM for this round. Thanks for trying on compiled RE.

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py
File bench/bench_util.py (right):

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode19
bench/bench_util.py:19: SETTING_RE = '([^\s=]+)(?:=(\S+))?'
How about PER_SETTING_RE?

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode33
bench/bench_util.py:33: SETTING_RE_COMPILED = re.compile(SETTING_RE)
Agreed.
On 2013/02/14 16:11:21, epoger wrote:
> Surprisingly, this still isn't any faster than it was before!
> 
> But at this point, I think it's worth committing, just to make it clear that
all
> of the regexes in here are constants.  None of them use different patterns as
> the code runs.

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode180
bench/bench_util.py:180: bench_dic, layout_dic, representation)
Just a thought: each line can be either per-tile or not, but not both. If we use
non-RE ways to differentiate the two first (say, if the line contains ' out of
'), we can potentially cut the time almost in half.

[epoger, 2013-02-14 18:34:25]

Will commit at patchset 3...

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py
File bench/bench_util.py (right):

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode19
bench/bench_util.py:19: SETTING_RE = '([^\s=]+)(?:=(\S+))?'
On 2013/02/14 17:43:31, benchen wrote:
> How about PER_SETTING_RE?

Done.

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode180
bench/bench_util.py:180: bench_dic, layout_dic, representation)
On 2013/02/14 17:43:31, benchen wrote:
> Just a thought: each line can be either per-tile or not, but not both. If we
use
> non-RE ways to differentiate the two first (say, if the line contains ' out of
> '), we can potentially cut the time almost in half.

That seems promising... unfortunately, the test data I have checked in for the
self-test is entirely per-tile.  I don't want to go messing with that code until
we can confirm that both kinds of lines are stil dealt with properly.

$ grep -v 'out of'
tools/tests/benchgraphs/Skia_Shuttle_Ubuntu12_ATI5770_Float_Bench_32/raw-bench-data/*
| grep -v 'running bench' | grep -v 'bench_pictures' | wc -l
0

[epoger, 2013-02-14 18:50:59]

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py
File bench/bench_util.py (right):

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode180
bench/bench_util.py:180: bench_dic, layout_dic, representation)
On 2013/02/14 18:34:25, epoger wrote:
> On 2013/02/14 17:43:31, benchen wrote:
> > Just a thought: each line can be either per-tile or not, but not both. If we
> use
> > non-RE ways to differentiate the two first (say, if the line contains ' out
of
> > '), we can potentially cut the time almost in half.
> 
> That seems promising... unfortunately, the test data I have checked in for the
> self-test is entirely per-tile.  I don't want to go messing with that code
until
> we can confirm that both kinds of lines are stil dealt with properly.
> 
> $ grep -v 'out of'
>
tools/tests/benchgraphs/Skia_Shuttle_Ubuntu12_ATI5770_Float_Bench_32/raw-bench-data/*
> | grep -v 'running bench' | grep -v 'bench_pictures' | wc -l
> 0

Holy mackerel!  I took a stab at this, just to see if the time savings would be
significant (even though I don't have all the test data I want yet)... and the
time dropped from 38 seconds to 3 seconds!

[benchen, 2013-02-14 19:55:12]

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py
File bench/bench_util.py (right):

https://codereview.appspot.com/7336043/diff/3001/bench/bench_util.py#newcode180
bench/bench_util.py:180: bench_dic, layout_dic, representation)
Really???!!! So that means all the time was wasted on trying to find an RE match
on lines that don't have a match.
On 2013/02/14 18:51:00, epoger wrote:
> On 2013/02/14 18:34:25, epoger wrote:
> > On 2013/02/14 17:43:31, benchen wrote:
> > > Just a thought: each line can be either per-tile or not, but not both. If
we
> > use
> > > non-RE ways to differentiate the two first (say, if the line contains '
out
> of
> > > '), we can potentially cut the time almost in half.
> > 
> > That seems promising... unfortunately, the test data I have checked in for
the
> > self-test is entirely per-tile.  I don't want to go messing with that code
> until
> > we can confirm that both kinds of lines are stil dealt with properly.
> > 
> > $ grep -v 'out of'
> >
>
tools/tests/benchgraphs/Skia_Shuttle_Ubuntu12_ATI5770_Float_Bench_32/raw-bench-data/*
> > | grep -v 'running bench' | grep -v 'bench_pictures' | wc -l
> > 0
> 
> Holy mackerel!  I took a stab at this, just to see if the time savings would
be
> significant (even though I don't have all the test data I want yet)... and the
> time dropped from 38 seconds to 3 seconds!